Details

Reviewers

Clément Foucault (fclem)
Brecht Van Lommel (brecht)

Maniphest Tasks

T77346: Intel UHD 620: Blender crashes at modifying material properties

Commits

rBf7d38e2e6426: Fix T77346: GPU Workaround Always Render Using Main Context

Summary

In Blender 2.90 EEVEE materials were refactored that introduced crashes on Intel
GPUs on Windows. The crash happened in the local_context_workaround that temporary
stored compiled materials in a binary form to reload it in the main GL context.

It has been tested that the workaround isn't needed anymore for HD6xx GPUs, but it
is still needed for HD4000.

After several unsuccesfull fixes we came to the conclusion that we could not support
the local context workaround and needed to come with a different workaround. The idea
of this patch is that in these cases there is only a single context that is used for
rendering. Threads that uses these contextes are guarded by a mutex and will block.

Impact on User Level:

Due to main mutex lock the UI freezes when rendering or baking or feel less snappy

Diff Detail

Repository: rB Blender

Event Timeline

Jeroen Bakker (jbakker) requested review of this revision.Jul 28 2020, 3:08 PM

Jeroen Bakker (jbakker) created this revision.

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Jul 28 2020, 3:10 PM

Jeroen Bakker (jbakker) added inline comments.Jul 28 2020, 3:13 PM

source/blender/draw/intern/draw_manager.c
1729	HD4000 Series
1732	these devices are officially supported
source/blender/gpu/intern/gpu_extensions.c
401	remove these two lines.

Updated comments

Jeroen Bakker (jbakker) retitled this revision from Fix T77346: GPU Workaround Always Render On Main Thread to [WIP] Fix T77346: GPU Workaround Always Render On Main Thread.Jul 28 2020, 3:27 PM

Fixed Motion Blur
Fixed Light Probes
Fixed Final Image Rendering

Harbormaster completed remote builds in B9306: Diff 27356.Aug 3 2020, 12:59 PM

Jeroen Bakker (jbakker) retitled this revision from [WIP] Fix T77346: GPU Workaround Always Render On Main Thread to Fix T77346: GPU Workaround Always Render On Main Thread.Aug 3 2020, 1:24 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)

Jeroen Bakker (jbakker) added a reviewer: Brecht Van Lommel (brecht).

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Aug 3 2020, 2:54 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)

To make preview renders and canceling animation renders work, and to do depsgraph evaluation in the render thread, this could be implemented a bit differently. Two ways:

Temporarily use the main thread context in the render thread. Use WM_job_main_thread_lock_acquire/release to ensure the main thread is not doing else at the same time.
Only run the part of that needs the OpenGL context on the main thread. Add a function WM_job_run_on_main_thread(void (*func)(void *args)), which would store that function pointer in a queue for the main thread, and then do a blocking wait until the main thread has run it.

To me that seems like it could make the hack more isolated and preserves more functionality.

Jeroen Bakker (jbakker) added a task: T77346: Intel UHD 620: Blender crashes at modifying material properties .Aug 3 2020, 4:34 PM

@Brecht Van Lommel (brecht) as we don't have access to the HW I think that WM_job_main_thread_lock_acquire/WM_job_main_thread_lock_release is a better option for now. The second option is harder to detect if we handled everything correctly.

Hmmm... here seems to be a new driver released for HD4000 https://downloadcenter.intel.com/product/81499/Intel-HD-Graphics-4000
I haven't seen any reports for this, but also cannot tell if the issues have been fixed in the driver.

The WM_job_main_thread_lock_acquire/WM_job_main_thread_lock_release has more impact due to how the current API is setup. We have to make sure that any of the next jobs aren't running :

WM_JOB_TYPE_RENDER
WM_JOB_TYPE_RENDER_PREVIEW
WM_JOB_TYPE_LIGHT_BAKE

It would be easier to do it the other way around (add global wm draw ticket mutex) and when rendering with eevee on the specific card we can add a ticket to one of them from the specific jobs.

My gut feeling tells me that we shouldn't do deferred shading for these cards, but still we have no way how to test. I am asking @Germano Cavalcante (mano-wii) if he could help out.

Couldn't we mark these specific drivers in GPU_platform and call it a day?
10.18.10.3, 10.18.10.4, 10.18.10.5, 10.18.14.4, 10.18.14.5 would then be marked GPU_SUPPORT_LEVEL_UNSUPPORTED and 20.19.15.4285 would be marked GPU_SUPPORT_LEVEL_LIMITED

In D8410#206135, @Jeroen Bakker (jbakker) wrote:

Hmmm... here seems to be a new driver released for HD4000 https://downloadcenter.intel.com/product/81499/Intel-HD-Graphics-4000
I haven't seen any reports for this, but also cannot tell if the issues have been fixed in the driver.

As far as I know these are just repackaged drivers with perhaps security fixes, but other than that they are not actually getting updates.

The WM_job_main_thread_lock_acquire/WM_job_main_thread_lock_release has more impact due to how the current API is setup. We have to make sure that any of the next jobs aren't running :

WM_JOB_TYPE_RENDER

WM_JOB_TYPE_RENDER_PREVIEW

WM_JOB_TYPE_LIGHT_BAKE

It would be easier to do it the other way around (add global wm draw ticket mutex) and when rendering with eevee on the specific card we can add a ticket to one of them from the specific jobs.

As far as I know the existing functions should work?

wm_job_main_thread_yield is called from the main loop, and the main thread will not do any OpenGL drawing will it is waiting for that lock. Nor will any other jobs using that lock do any OpenGL drawing at the same time.

Use a global mutex for the main context

Harbormaster completed remote builds in B9354: Diff 27431.Aug 5 2020, 11:55 AM

Removed debug line

Harbormaster completed remote builds in B9355: Diff 27432.Aug 5 2020, 11:56 AM

Fixed incorrect logic lightbake

Harbormaster completed remote builds in B9357: Diff 27434.Aug 5 2020, 12:00 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Aug 5 2020, 12:04 PM

Jeroen Bakker (jbakker) retitled this revision from Fix T77346: GPU Workaround Always Render On Main Thread to Fix T77346: GPU Workaround Always Render Using Main Context.Aug 5 2020, 12:06 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)

Seems way cleaner! Kudos for fixing that!

source/blender/draw/engines/eevee/eevee_lightcache.c
534 ↗	(On Diff #27434)	Style nitpick: Don't use vars if it is not reused.
555 ↗	(On Diff #27434)	Same here
683 ↗	(On Diff #27434)	Same here
source/blender/gpu/GPU_context.h
45 ↗	(On Diff #27434)	Maybe add a comment here to remember what it is for.

Fine from my side.

This revision is now accepted and ready to land.Aug 5 2020, 3:14 PM

Closed by commit rBf7d38e2e6426: Fix T77346: GPU Workaround Always Render Using Main Context (authored by Jeroen Bakker (jbakker)). · Explain WhyAug 5 2020, 3:46 PM

This revision was automatically updated to reflect the committed changes.

Jeroen Bakker (jbakker) marked 2 inline comments as done.

Jeroen Bakker (jbakker) added a commit: rBf7d38e2e6426: Fix T77346: GPU Workaround Always Render Using Main Context.

Fix T77346: GPU Workaround Always Render Using Main Context
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 27186

source/blender/draw/intern/draw_manager.c

source/blender/draw/intern/draw_manager_shader.c

source/blender/editors/render/render_internal.c

source/blender/gpu/GPU_extensions.h

source/blender/gpu/intern/gpu_codegen.h

source/blender/gpu/intern/gpu_codegen.c

source/blender/gpu/intern/gpu_extensions.c

source/blender/gpu/intern/gpu_init_exit.c

Fix T77346: GPU Workaround Always Render Using Main ContextClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 27186

source/blender/draw/intern/draw_manager.c

source/blender/draw/intern/draw_manager_shader.c

source/blender/editors/render/render_internal.c

source/blender/gpu/GPU_extensions.h

source/blender/gpu/intern/gpu_codegen.h

source/blender/gpu/intern/gpu_codegen.c

source/blender/gpu/intern/gpu_extensions.c

source/blender/gpu/intern/gpu_init_exit.c

Fix T77346: GPU Workaround Always Render Using Main Context
ClosedPublic
Actions

Revision Contents
Changeset List