GNOME Bugzilla – Bug 709827
Add API to control per-FB depth writing
Last modified: 2013-10-28 17:10:35 UTC
Created attachment 256918 [details] [review] Patch to allow controlling depth writing per-FB I've been looking at ways to improve the software rendering performance of gnome-shell, and one of the wins I found was to disable the depth and stencil buffers at the Cogl level. I contacted Neil Roberts about this, and one of the long-term solutions he offered was to disable depth buffer writing. In particular, this should save us the extra work during glClear(). I've written a patch against git master to add the requisite API. Since I don't know which version this would appear in at the earliest, I just made the doc strings say "since 2.0".
Thanks for the patch, it looks really good. There were just a few minor things I noted... I think it could be worthwhile to bail out from cogl_framebuffer_set_depth_write_enabled in the case where we can see the enabled state isn't changing. It looks like you followed the style of _set_color_mask here and actually we should have made that try and bail out too. I've just sent a patch to fix the _set_color_mask api to the list here: http://lists.freedesktop.org/archives/cogl/2013-October/001439.html For the since 2.0 annotations, we've started to avoid using open ended 2.0 version annotations like that (though we still have some annotations that should probably be fixed) and instead we just refer to the next stable release we are working towards which is 1.18 now. If it's not too much trouble, could you perhaps write a basic conformance test for this new api, perhaps just tweaking the existing test-depth-state.c to verify the api?
Created attachment 257970 [details] [review] Patch to allow controlling depth writing per-FB Here's the updated patch. I caught an issue that would trigger a g_warn_if_reached() in cogl-framebuffer-gl.c, too. I'm pretty sure there's nothing we're supposed to be doing there, so I just made it a no-op.
Comment on attachment 257970 [details] [review] Patch to allow controlling depth writing per-FB Thanks a lot for updating the patch, including adding a conformance test case, it looks really good to me and I've landed it on the master and cogl-1.18 branches. Just one minor note; I found that your patch wasn't formatted using git-format-patch which meant that it couldn't be directly applied using git-am without editing it to have From: and Subject: fields. This wasn't a big problem but thought you might want to know that it's easier for others to apply your patches with all of the meta data from git if you use git-format-patch(1).