the crash itself is easily fixed by changing one line:
  cairo_surface_flush (impl->cairo_surface);
to:
  cairo_surface_flush (window->current_paint.surface);

In non-GL case these surfaces are the same, but when OpenGL is used, impl->cairo_surface remains NULL, while window->current_paint.surface does not.

That, however, results in GL not drawing anything (the rest of the widgets are drawn normally). I'm still investigating.

It works with software fallback (GDK_GL=software-draw-gl), so that's something.

After some reading, my current theory is that the problem originates with the way OpenGL context is created. It's based on a DC of a window. It doesn't seem to be well-documented, but my understanding is that you can't draw on a DC of a layered window. And my layered window-drawing code does not, it creates a DIB-backed cairo surface, gets DC from that, and feeds it to UpdateLayeredWindow().

So one way to fix OpenGL rendering would be to do something similar - instead of basing OpenGL context on the DC of our application window, we could use some other DC. However, the things i've googled up[1] suggest that the best results are achieved by using an actual window DC, which means creating a hidden window, rendering onto it using its DC, then extracting the rendered image somehow (ideally by just feeding DC of the window to UpdateLayeredWindow()).

[1] https://stackoverflow.com/questions/21672273/how-can-i-off-screen-render

Hi LRN,

Thanks for the info... it's time that we need to look into using PBO's and FBO's for this purpose.  Need to look into a tutorial how that will work with UpdateLayeredWindow(), possibly like the one at [1].

From experience while doing my code in gdkglcontext-win32.c, we can't just use a DC that taken from CreateCompatibleDC(), as it would not be enough to serve our purposes here for OpenGL, as it would kick us hard back into OpenGL 1.1, which is clearly not enough for us.

[1]: http://allanpetersen.com/2010/04/fbo-and-bitmap-layer-demo/

With blessings, thank you!

It has been pointed out to me that drawing into some kind of buffer and then copying that into cairo surface is possible, but is also non-optimal.

In case of layered windows the most optimal path possible (constrained only by the nature of layered windows) is to have a fully-functional DC (i.e. a DC we can get from a window, not a memory DC (for the reasons outlined in previous comment); hence the abovementioned idea for creating an invisible window), draw everything into it (both cairo and OpenGL can do that naturally) directly, then feed that context to UpdateLayeredWindow() as the source of the window contents.
Question is: would this work? Specifically, the part where you feed one window's context to UpdateLayeredWidndow() for another window.

By the way, while writing this comment it occurred to me that i should have probably done something similar to pure-cairo layered window rendering (i.e. drawing directly on the cache surface). I probably could have created a DC myself, then made a surface out of it with cairo_win32_surface_create_with_format(). I guess performance was not the primary objective at the moment...

Did some testing: created 2 windows - one layered, one normal. Then used W32 API to draw on normal window DC and then used that DC as a source of content for UpdateLayeredWindow() for the layered window.

That scheme only worked when normal window was made visible. Googling showed that other people had the same problem.

Generally, the advice is to use FBO for accelerated offscreen rendering (AFAICS, GDK already does that, at least in some cases; the docs say that FBO is used for rendering with no alpha; GDK also uses FBO for software fallback).
The question is then "how to get the data out of FBO?". Google says that bare glReadPixels() (which is what GDK currently uses for software fallback) is slow and advises to use PBO. From what i'm reading, PBO allows glReadPixels() to work asynchronously. Thing is, i'm not sure what kind of activity we could put between glReadPixels() and glMapBuffer() to fill the time OpenGL needs to transfer the data. If that is not done, and glReadPixels() is followed immediately by glMapBuffer(), then we get exactly what software fallback already does. gdkgears with software fallback is ~2x slower on my machine (although it suspiciously renders at exactly 30.0 FPS, while non-layered non-fallback version renders at 58.8, so maybe there's more to that).

Another thing of note is that GDKGL software fallback uses an image surface, which adds an extra copying operation.

In theory all of this sounds quite bad and scary to me. I'm tempted to just say "don't use WS_EX_LAYERED for windows that have GL rendering, or use software fallback" and be done with it.

Hi LRN,

Thanks, I am also tempted to say the same thing, after looking at things last night--the fast way to fix this is to not use layered Windows for GL stuff--the software fall back doesn't work for me, for instance, and it's going to be slower.  Wonder whether there are any way to disable CSD for GL widgets though.  PBO, as I heard when doing the Windows GDK GL support, is also more like deprecated in favor of FBO's, and it was not something fun when trying to use it for the support for GL GDK on Windows.

Thanks, with blessings, and cheers!

(In reply to Fan, Chun-wei from comment #6)
> the fast way to fix this is to not use layered Windows for GL
> stuff
Yeah, i'll investigate that as the simplest solution. I think it's sufficient to check that window->impl_window->gl_paint_context != NULL, this is what gdkwindow does. If it's != NULL, don't use layered stuff.

> the software fall back doesn't work for me, for instance
This is bad. Software fallback should definitely be working.
> Wonder whether there are any way to disable CSD for GL
> widgets though.
CSD is the property of a window, not a widget. If you meant "to disable CSD on a window that has GL widgets in it", well, that is...um...i don't know, actually. GDK has that data, but GTK does not, and it's GTK that decides whether a window is going to be CSD or not, and since it doesn't know about gl status of a GDK window, it can't decide. Also, some windows are explicitly requested to be CSD by their designers, and there's nothing we can do about that.

> PBO, as I heard when doing the Windows GDK GL support, is
> also more like deprecated in favor of FBO's
I think there's some kind of misunderstanding (because i've had the same misunderstanding for some time while googling this stuff).
FBO == Framebuffer objects. It's a way to do hardware-accelerated rendering that draws offscreen (i.e. not requiring a window or any other kind of special surface).
PBO == Pixel Buffer Objects. It's a way to do asynchronous glReadPixels(), in a nutshell.
P-buffer == Pixel Buffer. It's a deprecated way to do the same thing that FBO does.

Another note is that GDKGL always uses an image surface to combine GL-drawn stuff and cairo-drawn stuff.
That is, window->current_paint.surface is always an image surface when there's a GL widget. So *any* OpenGL rendering, as far as i understand, always renders into that image surface (supposedly; W32 stuff still does something with window DCs, i don't know what). This doesn't strike me as very optimal (performance-wise) to begin with.
I'll ask ebassi about it.

Created attachment 323135 [details] [review]
GDK-Win32: Disable layered Windows when using OpenGL

Hi,

This is my take at tackling this, for now.  When we are creating a GdkGLContext, disable layered Windows...

With blessings, thank you!

Created attachment 323139 [details] [review]
GDK W32: Don't use WS_EX_LAYERED for windows with GL widgets

My take on this. Adds yet another field to the impl - a counter
that suppresses the application of WS_EX_LAYERED style to
the window.
GL context increases and decreases this counter when it's created
and disposed respectively, and does a no-op with window hints
to trigger update_style_bits (ugly, i know).

Created attachment 323240 [details] [review]
Disable layered Windows when using OpenGL

Hi LRN,

I looked at your patch and I thought the things there should also go into consideration, so I came up with a new patch that:

-Optimizies a few things--we don't need to trigger update_style_bits()
 that much, only when impl->suppres_layered gets above (or to) 0.
-Increment impl->suppres_layered only when we realize the GL context,
 as that is when the HGLRC (Windows GL Context) is really created.
-The cairo_flush_surface() call runs properly as it is with these changes.

With blessings, thank you!

Review of attachment 323240 [details] [review]:

See the nitpick. Otherwise, i think, it's OK.

::: gdk/win32/gdkglcontext-win32.c
@@ +530,3 @@
+   */
+  if (suppressed_layered_windows == 0)
+    gdk_window_set_type_hint (window, gdk_window_get_type_hint (window));

Um...can't we just check if (impl->suppress_layered == 1) and don't store it in suppressed_layered_windows above?

Hi LRN 

Hmm, that's true... doh.  I will do a new patch a bit later, since I need to head out for the evening here.

With blessings, thank you!

Created attachment 323288 [details] [review]
Disable layered Windows when using OpenGL (take ii)

Hi,

Here comes the new patch, which:
-Addressed the nitpick
-Make comments a bit more clear

With blessings, thank you!

Review of attachment 323135 [details] [review]:

Okay

The patch is reviewed as "commit now", and that is what you should do.

That said, i have 2 unrelated things to say:
1) We should probably create a function that has one purpose - call update_style_bits(). We control the implementation, after all, so why bother with indirect approach with window hints?

2) OpenGL area demo in gtk3-demo, when run with this patch applied, does not work 100% well - it doesn't redraw the GL area when window is resized. This didn't happen before (though my ability to test this "before" is limited to some old builds that i have laying around, so i'm not sure which commit broke it).

Hi LRN,

(In reply to LRN from comment #15)
> The patch is reviewed as "commit now", and that is what you should do.

Thanks, I just pushed it as 3f190e0.

> That said, i have 2 unrelated things to say:
> 1) We should probably create a function that has one purpose - call
> update_style_bits(). We control the implementation, after all, so why bother
> with indirect approach with window hints?

Agreed.  This should reduce overhead a bit more.
 
> 2) OpenGL area demo in gtk3-demo, when run with this patch applied, does not
> work 100% well - it doesn't redraw the GL area when window is resized. This
> didn't happen before (though my ability to test this "before" is limited to
> some old builds that i have laying around, so i'm not sure which commit
> broke it).

Hmm, I tried running gtk3-demo with GTK_CSD=0 set, and the GL area redraws normally... It does seem to me that area becomes transparent after the resize, so maybe the layered window is placed on top of the GL area.  If you do move any of the 3 sliders though, the GL area does show and update normally.

The other thing that *might* have something to do with this is that the gdkgears test program does redraw the GL areas, but the main GL area becomes white during the resize drag but then is normal when the resize completes.

This seems to me an issue in the CSD-related items.

Let's close this bug and open new ones for these issues.

With blessings, thank you, and cheers!