Page MenuHome
Differential D3057

GHOST/DRW: Add interface to manage offscreen opengl contexts
AbandonedPublic
Actions

Authored by Clément Foucault (fclem) on Feb 10 2018, 3:34 PM.

Details

Reviewers
Sergey Sharybin (sergey)
Campbell Barton (campbellbarton)
Dalai Felinto (dfelinto)
Brecht Van Lommel (brecht)
Summary

This is an attempt to create a separate context for the drawmanager.

This has two benefits:

  • We can run this context in parallel with the UI context. (multithreaded F12 render)
  • The Batches' VAOs are creating under this only context making multiple window viewport work (because every viewport rendering uses this context).

All resources are shared with all contexts (as always).

For now only Linux and Windows are supported.
(Windows support is courtesy of @Germano Cavalcante (mano-wii))

There is still crashes and instabilities due to the old way of managing contexts and FBO / VAO. Freeing is particulary bad.
This does not fix VAOs used under each UI windows contexts (i.e. the normal widget [see sky texture node]).

To reviewers: I want to know if this approach is not too much invasive in respect to the WM code. And if the GHOST part makes sense. Also not sure about all the naming of functions.

Diff Detail

Repository
rB Blender
Branch
temp-drawcontext
Build Status
Buildable 1193
Build 1193: arc lint + arc unit

Event Timeline

Clément Foucault (fclem) created this revision.Feb 10 2018, 3:34 PM
Clément Foucault (fclem) retitled this revision from GHOST: Add interface to manage offscreen opengl contexts. to GHOST/DRW: Add interface to manage offscreen opengl contexts.Feb 10 2018, 3:51 PM
Clément Foucault (fclem) edited the summary of this revision. (Show Details)
Clément Foucault (fclem) added reviewers: Sergey Sharybin (sergey), Campbell Barton (campbellbarton), Dalai Felinto (dfelinto), Brecht Van Lommel (brecht).
Clément Foucault (fclem) added a project: BF Blender: 2.8.
Clément Foucault (fclem) added a subscriber: Germano Cavalcante (mano-wii).
Clément Foucault (fclem) mentioned this in T51736: Drawing issues when running in multiple windows.Feb 10 2018, 4:01 PM
Brecht Van Lommel (brecht) added a comment.Feb 10 2018, 4:44 PM

Overall this make a lot of sense to me.

Would the eventual goal be to do all drawing into offscreen framebuffers? If each area or region has its own framebuffer, we could get rid of the triple buffer textures and give the windows just an RGB buffer without any Z/A/multisample storage. It's something I wanted to try at some point, and it would solve issues like VAOs for the normal widget drawing.

intern/ghost/intern/GHOST_SystemWin32.cpp
315

It would be good to have a comment explaining this, it's not obvious to me why an offscreen context would be tied to a window. Is it an API limitation? Maybe it determines with GPU is used if there are multiple GPUs used for different monitors?

It also seems problematic for background mode OpenGL render, but I'm not sure if this code would be reused for that.

323

Indentation seems too deep here.

331

I guess this false will need to be replaced by some variables?

source/blender/draw/intern/draw_manager.c
383

Only write locks are used for this read-write mutex, so unless you are planning to use read locks you could use a regular mutex instead.

3653–3656

Maybe add a DRW_ogl_select_ctx_enable() and use it here? Just to keep that locking logic together a bit.

4199

I would prefer to have the full name DRW_opengl_context_create() here for clarity, it's not so long and abbreviations are not always obvious.

source/blender/windowmanager/intern/wm_window.c
99

Any reason we can't use wm->winactive here?

If it's to avoid passing context, you can use wmWindowManager *wm = G.main->wm.first;. If it has a different meaning, that seems confusing and the name/comment should be more clear on what the difference is.

2060

Maybe use WM_opengl_context_create(), it's not so obvious otherwise.

2062

And this one could also be GHOST_CreateOpenGLContext, so it's a consistent name throughout.

Clément Foucault (fclem) added a comment.Feb 11 2018, 12:26 AM

For Windows API check with @Germano Cavalcante (mano-wii)

For my part I agree with the comments in general but ...

source/blender/windowmanager/intern/wm_window.c
99

I used it because winactive was not set. But in fact I should have used windrawable. Is it ok to use wm.first? can there be multiple window manager? can there be none?

2062

I find the naming of GHOST_CreateOpenGLContext pretty redundant for GHOST and the context could be tied to the window (not in the api for now). This is to differentiate them. The offscreen context has no surface attached to it. And maybe we will have a background context creation in the future GHOST_CreateBackgroundContext. Since it's GHOST the context is obviously OGL, right?.

Clément Foucault (fclem) mentioned this in rB5e8030e68251: DRW Opengl Context: Adress comments on D3057.Feb 11 2018, 2:47 AM
Brecht Van Lommel (brecht) added inline comments.Feb 12 2018, 5:20 PM
source/blender/windowmanager/intern/wm_window.c
99

It's safe to use wm.first, there is always exactly one window manager, and the same code is used in other places.

2062

I don't mind it being called GHOST_CreateOffscreenContext. I just found it a bit confusing that the same thing was called three slightly different names in three different modules. Maybe it can be "offscreen context" everywhere then. It's just a small nitpick though.

Brecht Van Lommel (brecht) mentioned this in D3061: WM: new offscreen window draw method to replace existing ones..Feb 13 2018, 4:43 PM
Clément Foucault (fclem) updated this revision to Diff 10055.Feb 22 2018, 3:04 PM
  • DRW Opengl Context: Adress comments on D3057
  • Merge branch 'blender2.8' of git@git.blender.org:blender.git into temp-drawcontext
  • GHOST: Style, add comments, and make sure that the system has at least one window
  • Create an offscreen context with own hDC
  • GHOST: offscreen context support for macOS.
  • GHOST: Remove exception from multiple context on intel cards
  • Eevee: Fix SSR in multiviewport.
  • DRW: Fix DRW_draw_select_loop and DRW_draw_depth_loop
  • Merge branch 'blender2.8' of git@git.blender.org:blender.git into temp-drawcontext
  • Merge remote-tracking branch 'origin/blender2.8' into temp-drawcontext
  • DRW: Use new gawain context.
  • Merge remote-tracking branch 'origin/blender2.8' into temp-drawcontext
  • Eevee: Fix SSR not appearing in render.
  • Fix Edit Mode selection triggering an OpenGL error.
  • DRW: Fix selection & cursor autodist not working after rendering.
  • Merge remote-tracking branch 'origin/blender2.8' into temp-drawcontext
  • Merge remote-tracking branch 'origin/blender2.8' into temp-drawcontext
Brecht Van Lommel (brecht) requested changes to this revision.Feb 22 2018, 3:50 PM
Brecht Van Lommel (brecht) added inline comments.
intern/ghost/intern/GHOST_SystemWin32.cpp
315

Solving this will require passing GHOST_GLSettings to GHOST_CreateOpenGLContext(). Not sure how much you are relying on OpenGL debugging, but seems like keeping that could be somewhat important.

source/blender/draw/intern/draw_manager.c
384

Rename to g_ogl_context_lock or something like that?

source/blender/editors/render/render_opengl.c
850

Maybe oglrender->fx should be created and destroyed in the context as well? It seems safer to just do it for all the OpenGL data structures anyway, even if currently they can be shared.

source/blender/editors/space_view3d/view3d_draw.c
2136

Disable the context here.

source/blender/gpu/intern/gpu_viewport.c
527–532

DRW_opengl_context_disable() should be after the glEnable / glDisable I think? At least GPU_viewport_bind() doesn't seem to do any OpenGL state changes before calling DRW_opengl_context_enable().

source/blender/windowmanager/intern/wm_init_exit.c
212

DRW_opengl_context_destroy() doesn't seem to be called anywhere.

source/blender/windowmanager/intern/wm_window.c
1043–1052

It seems more clear to me if the immediate mode and gawain calls that are here go to the DRW_opengl_context functions, Now it's kind of spread out.

Also not sure if this TODO is important to fix still.

This revision now requires changes to proceed.Feb 22 2018, 3:50 PM
Brecht Van Lommel (brecht) added inline comments.Feb 22 2018, 4:03 PM
source/blender/windowmanager/intern/wm_window.c
1043–1052

Actually, wm_window_make_drawable does the same stuff so I guess it's ok. Just didn't expect that stuff in the WM.

Clément Foucault (fclem) updated this revision to Diff 10056.Feb 22 2018, 4:37 PM
  • GWN: Fix immediate mode when closing a window.
Clément Foucault (fclem) updated this revision to Diff 10057.Feb 22 2018, 8:17 PM
  • Merge remote-tracking branch 'origin/blender2.8' into temp-drawcontext
Brecht Van Lommel (brecht) added a comment.EditedFeb 25 2018, 4:20 AM

I've committed fixes for macOS in rB813f1c603f64: Fixes for offscreen drawing context on macOS..

Mainly I needed to add a glFlush() call. I don't know if this is just a macOS quirk or what, but I've put it behind #ifdef __APPLE__ in case it has a performance impact on other platforms.

Things seem to be fully working now on macOS + Intel Iris. Some of my comments from D3057#72681 should still be addressed though.

Brecht Van Lommel (brecht) abandoned this revision.Apr 3 2018, 4:28 PM

This was committed in rB0940e89e604d and rB13261304a331b4c.

Revision Contents
Changeset List

PathSize
intern/
ghost/
1 line
25 lines
72 lines
15 lines
intern/
21 lines
3 lines
7 lines
14 lines
175 lines
15 lines
105 lines
20 lines
98 lines
39 lines
source/
blender/
draw/
5 lines
intern/
7 lines
106 lines
editors/
render/
9 lines
8 lines
space_view3d/
3 lines
12 lines
gpu/
2 lines
intern/
11 lines
6 lines
windowmanager/
1 line
4 lines
intern/
5 lines
35 lines
1 line

Diff 10008

View Options

intern/ghost/CMakeLists.txt

Loading...
View Options

intern/ghost/GHOST_C-api.h

Loading...
View Options

intern/ghost/GHOST_IContext.h

Loading...
View Options

intern/ghost/GHOST_ISystem.h

Loading...
View Options

intern/ghost/intern/GHOST_C-api.cpp

Loading...
View Options

intern/ghost/intern/GHOST_Context.h

Loading...
View Options

intern/ghost/intern/GHOST_ContextGLX.cpp

Loading...
View Options

intern/ghost/intern/GHOST_ContextWGL.h

Loading...
View Options

intern/ghost/intern/GHOST_ContextWGL.cpp

Loading...
View Options

intern/ghost/intern/GHOST_SystemWin32.h

Loading...
View Options

intern/ghost/intern/GHOST_SystemWin32.cpp

Loading...
View Options

intern/ghost/intern/GHOST_SystemX11.h

Loading...
View Options

intern/ghost/intern/GHOST_SystemX11.cpp

Loading...
View Options

intern/ghost/intern/GHOST_WindowWin32.cpp

Loading...
View Options

source/blender/draw/DRW_engine.h

Loading...
View Options

source/blender/draw/intern/draw_cache.c

Loading...
View Options

source/blender/draw/intern/draw_manager.c

Loading...
View Options

source/blender/editors/render/render_internal.c

Loading...
View Options

source/blender/editors/render/render_opengl.c

Loading...
View Options

source/blender/editors/space_view3d/view3d_draw.c

Loading...
View Options

source/blender/editors/space_view3d/view3d_view.c

Loading...
View Options

source/blender/gpu/GPU_batch.h

Loading...
View Options

source/blender/gpu/intern/gpu_batch_presets.c

Loading...
View Options

source/blender/gpu/intern/gpu_viewport.c

Loading...
View Options

source/blender/windowmanager/CMakeLists.txt

Loading...
View Options

source/blender/windowmanager/WM_api.h

Loading...
View Options

source/blender/windowmanager/intern/wm_init_exit.c

Loading...
View Options

source/blender/windowmanager/intern/wm_window.c

Loading...
View Options

source/blender/windowmanager/wm_window.h

Loading...