Proposal: Ensure all required GL states are correct in beginFrame

[memononen]

Proposal: Ensure all required GL states are correct in beginFrame

[Nairou]

I think these additions would be great, but I don't think the
glViewport/glClear lines should be on by default (or even included, since
they're obvious). NanoVG shouldn't assume you're using a framebuffer, since
it didn't set one up itself, and so shouldn't potentially wipe the
backbuffer.

Another state to add to the list is glFrontFace(GL_CCW). I've run into that
issue before.

On Sun, Apr 13, 2014 at 4:53 PM, paniq [email protected] wrote:

It appears NanoVG doesn't set all states required for it to render
correctly, which increases the amount of boilerplate needed on the client
side, and also causes all kinds of hard to diagnose side effects if the
user is making use of other blackbox GL components.

I'm not sure why this was done, perhaps to make multiple subsequent draw
calls more efficient, but in practice, this should not make any noticeable
differences in performance.

Among the states mentioned in the docs, which are
glEnable(GL_CULL_FACE)
glCullFace(GL_BACK)
glEnable(GL_BLEND)
glDisable(GL_DEPTH_TEST)

I also had to take care that these states are initialized as follows
(still probably not a complete list):
glDisable(GL_SCISSOR_TEST)
glColorMask(GL_TRUE,GL_TRUE,GL_TRUE,GL_TRUE)
glStencilMask(0xffffffff)
glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP)
glStencilFunc(GL_ALWAYS, 0, 0xffffffff)

And then there are other obvious ones, which are
glViewport(0,0,frame_width,frame_height)
glClearColor(0,0,0,0)
glClear(GL_COLOR_BUFFER_BIT | GL_STENCIL_BUFFER_BIT)

I would like to propose that the alpha_blend argument in beginFrame is
converted to a general flags field, NVG_PREMULTIPLIED_ALPHA and
NVG_STRAIGHT_ALPHA control bit 0, and a new NVG_NAKED_FRAME field is
introduced, which, when set, does not make any of the above calls; so
the default would be that all states are set correctly and the framebuffer
is cleared for drawing.

For more granular control, if desired, NVG_NAKED_FRAME could also be a
composite of three bits, which is at most NVG_KEEP_STATES (no state
changes), NVG_KEEP_VIEWPORT (no viewport change) and NVG_KEEP_FRAMEBUFFER
(no clear).

If none or not all of the proposed changes find their way into the
codebase, I'd appreciate if the docs would be expanded by the additionally
required boilerplate so others don't run into the same problems.

Reply to this email directly or view it on GitHubhttps://github.com//issues/59
.

[paniq]

Good one. Btw: You're always using a framebuffer.

[memononen]

This is excellent stuff, some of it definitely goes in and all of it should be documented. I was expecting to run into some of these issue as I don't usually stumble upon it on my test cases, since I tend to set them up the same way.

I was thinking about using push/pop attrib to make sure that there as little side effects as possible, but of course it is deprecated these days.

I'm not sure if there is anything to gain by not setting the correct initial state, but I would like to leave as few side effects as possible. Is there a reasonable (and not too slow) way to query the GL state per frame?

I'd like to keep clear & viewport on client side and document it (or at least not make them default). My rationale is that I expect nanovg to be generally used for rendering debug overlays, HUDs and UIs. But I'm very biased as that is my use case :) What do you guys think?

I would also love to query the stencil bits in begin frame and spit a warning if there's none, this seems to be a common case people stumble upon. I wonder if that is too expensive, though?

[paniq]

Is there a reasonable (and not too slow) way to query the GL state per frame?

It's generally said that glGet causes synchronization so no. But there's always something on the driver side that makes superfluous calls a non-op.

I'd like to keep clear & viewport on client side and document it (or at least not make them default). My rationale is that I expect nanovg to be generally used for rendering debug overlays, HUDs and UIs. But I'm very biased as that is my use case :) What do you guys think?

I generally tend to render stuff into framebuffers (rendertargets) first because that means the texture can be cached over multiple frames, re-used, shaded, projected, etc. and it also doesn't mess with the state of the scene, which can just be done in a flush, without intermissions. I intend to do all texture drawing with it, which includes HUD and UI's, but also more geometric procedural textures. I also think that wanting to do a lot of direct drawing into scenes will cause more complications: e.g. NanoVG has currently no 3D matrix (no global matrices in GL3.2+ anymore), and should it really have one?

IMO not worth bothering, for the reasons mentioned, and also beyond the scope of what I would expect of the library, which is to be a thinner, faster and GL-native version of Cairo (which HTML5 Canvas is also aping, and which itself is probably heavily inspired by Microsofts GDI+, which also has a great API). But I suppose our expectations might diverge.

I would also love to query the stencil bits in begin frame and spit a warning if there's none, this seems to be a common case people stumble upon. I wonder if that is too expensive, though?

Yes it is. But you could have a diagnostic function only called by users during debugging, that analyses the current state and gives an extensive list of what is wrong (could probably use the new GL debug output facilities for that).

[paniq]

Update for the diagnostic function: pass NVG_DEBUG or something either on context creation or on beginFrame, and that would then do what I described.

Then again, why have another conditional if there can be just another function. Your pick I guess ;)

[Nairou]

I totally agree about rendering UI to a texture, I'll probably move back to
that eventually as well. However, Mikko talked about reducing side effects
in using NanoVG, and having it clear a framebuffer that it didn't create
would seem to me to be a big "WTF happened?" side effect. If I'm going to
render the UI to a texture, I've already got the state changes setup before
and after for using it. Having some of those framebuffer state changes
hidden because NanoVG is automatically handling it is, I think, a bad idea.

On Sun, Apr 13, 2014 at 5:51 PM, paniq [email protected] wrote:

Update for the diagnostic function: pass NVG_DEBUG or something either on
context creation or on beginFrame, and that would then do what I described.

Reply to this email directly or view it on GitHubhttps://github.com//issues/59#issuecomment-40321073
.

[paniq]

Which is why those skip flags are important - it should always be possible to go in naked. But there has to be a dumb case so I can just get going, and because it is dumb, it has to be the default. It's neat that I have all this power, but I don't want to be forced to exercise it (plus, the dumb case is a good source reference for anyone looking to handle that part manually, so doubles as documentation. -- as Don Norman says, put the knowledge in the world, not in the mind)

I agree that the clear isn't appropriate for a framebuffer that it doesn't manage; it would be nice here to have surface functions that can create and configure a working framebuffer, stencil and all, again, doubling as doc for anyone wanting to do this by foot. Noob/high-level functions are helpful to get started, as long as they give you a way out when the project grows.

[memononen]

I like the debug flag idea (1). There are current bunch of glGetErrors() sprinkled around too, which could be put behind that flag too. I think that covers the checking the correct states and stencil support.

I think begin frame should always set the state it expects (2), the reason being that the code will need to change states anyway, so why not put the right states in the beginning? If it becomes a perf bottleneck (which I doubt), we could allow the user to pass in his current state and "double buffer" the changes. What do you think about this?

Lastly there are two kinds of side effects, one is the mess the nanovg leaves to the opengl state, the other one is unexpected results (viewport & clear). My current stab at things is that we'll document the states nanovg sets (3), and maybe have saveGLState()/restoreGLState() for the case when you just want to try things out (4). I'm still a little unsure about viewport and clear (5), but I think I just have to try and see.

I think points 1, 2 and 3 are something that we should just do. Point 4 is something I'll consider if need arises (or it may actually come as side effect of 1), and point 5 is something that needs a little experimenting to get the feeling right.

As for the flag naming I generally prefer names which are adding, that is NVG_SETUPVIEWPORT, NVG_CLEARFRAMEBUFFER, etc.

@paniq can you elaborate a bit more on the surface functions?

[paniq]

I think points 1, 2 and 3 are something that we should just do. Point 4 is something I'll consider if need arises (or it may actually come as side effect of 1), and point 5 is something that needs a little experimenting to get the feeling right.

yes. I personally have no use for 4, 3 is nice to have.

As for the flag naming I generally prefer names which are adding, that is NVG_SETUPVIEWPORT, NVG_CLEARFRAMEBUFFER, etc.

Me too, but the problem here is that the simple, dumb "it just works" cases then require extra flags, and that's a little baffling. Whether that functionality is "adding" or not is actually an implementation detail; as long as the flag itself is not exclusive ("NO_XYZ"), but inclusive (which is why I suggested "KEEP_XYZ"), it's still "adding".

@paniq can you elaborate a bit more on the surface functions?

Cairo does not only let you create a drawing context, but initializes the drawing context with a surface it's drawing to. That surface can be e.g. a GL buffer, an SVG or a bitmap in memory; So, it's implicitly selecting the backend too.

In this case, there would only be one kind of surface type, and it would be implemented as a framebuffer with two textures, one for color, one for stencil. Because the context knows exactly what it's dealing with, it doesn't have to feel shy doing a proper clear & initializing the viewport, and can even support a function that allows extracting the data from memory; this, in part, for folks who don't want to care about GL at all, and use NanoVG exclusively to do graphics.

By the way, with the current implementation, I noticed that there seems to be literally no reason at all why there should be more than one context active at any time; The only reason I can think of is a second GL context (which is generally not advised). Would it then not be OK to make the context implicit and thread local (as GL does), and remove the context pointer from nearly all functions?

[dougbinks]

Note on the context state - this is worth moving to another issue if
needed. I'm considering using NanoVG in a multithreaded fashion as for the
gl3buf backend all the calls between begin & end do no OpenGL calls but do
some CPU work so could be usefully moved to another thread. This would mean
it would be useful for me to use multiple contexts.

On 14 April 2014 09:02, paniq [email protected] wrote:

I think points 1, 2 and 3 are something that we should just do. Point 4 is
something I'll consider if need arises (or it may actually come as side
effect of 1), and point 5 is something that needs a little experimenting to
get the feeling right.

yes. I personally have no use for 4, 3 is nice to have.

As for the flag naming I generally prefer names which are adding, that is
NVG_SETUPVIEWPORT, NVG_CLEARFRAMEBUFFER, etc.

Me too, but the problem here is that the simple, dumb "it just works"
cases then require extra flags, and that's a little baffling. Whether that
functionality is "adding" or not is actually an implementation detail; as
long as the flag itself is not exclusive ("NO_XYZ"), but inclusive (which
is why I suggested "KEEP_XYZ"), it's still "adding".

@paniq https://github.com/paniq can you elaborate a bit more on the
surface functions?

Cairo does not only let you create a drawing context, but initializes the
drawing context with a surface it's drawing to. That surface can be e.g. a
GL buffer, an SVG or a bitmap in memory; So, it's implicitly selecting the
backend too.

In this case, there would only be one kind of surface type, and it would
be implemented as a framebuffer with two textures, one for color, one for
stencil. Because the context knows exactly what it's dealing with, it
doesn't have to feel shy doing a proper clear & initializing the viewport,
and can even support a function that allows extracting the data from
memory; this, in part, for folks who don't want to care about GL at all,
and use NanoVG exclusively to do graphics.

By the way, with the current implementation, I noticed that there seems to
be literally no reason at all why there should be more than one context
active at any time; The only reason I can think of is a second GL context
(which is generally not advised). Would it then not be OK to make the
context implicit and thread local (as GL does), and remove the context
pointer from nearly all functions?

Reply to this email directly or view it on GitHubhttps://github.com//issues/59#issuecomment-40337989
.

[paniq]

Just found another state that needs to be correct upon entering BeginFrame:

glActiveTexture(GL_TEXTURE0)

[memononen]

I have implemented the state setting in the latest nanovg_gl3buf.h backend.

I was planning to add the dirtied GL state into the readme.md, do you think it is a good place?

I'll move forward with the debug flag as my time permits.

[memononen]

I commented the states changed by the GL backend in README.md. Can you please comment on it? Is in the form that is easy to digest when you read it, is something missing?

[paniq]

Looking good. :)