Details

Reviewers

Brecht Van Lommel (brecht)
Jeroen Bakker (jbakker)

Maniphest Tasks

T93350: Cycles renders shows black during rendering huge resolutions

Commits

rCd29923554a7a: Fix T93350: Cycles renders shows black during rendering huge resolutions
rCd3a486d3fb2d: Fix T93350: Cycles renders shows black during rendering huge resolutions
rBae28d90578be: Fix T93350: Cycles renders shows black during rendering huge resolutions
rB5e37f70307bd: Fix T93350: Cycles renders shows black during rendering huge resolutions

Summary

The root of the issue is caused by Cycles ignoring OpenGL limitation on
the maximum resolution of textures: Cycles was allocating texture of the
final render resolution. It was exceeding limitation on certain GPUs and
driver.

The idea is simple: use multiple textures for the display, each of which
will fit into OpenGL limitations.

There is some code which allows the display driver to know when to start
the new tile. Also added some code to allow force graphics interop to be
re-created. The latter one ended up not used in the final version of the
patch, but it might be helpful for other drivers implementation.

The tile size is limited to 8K now as it is the safest size for textures
on many GPUs and OpenGL drivers.

Diff Detail

Repository

rB Blender

Branch

cycles_tiled_display (branched from master)

Build Status

Buildable 18993
Build 18993: arc lint + arc unit

Event Timeline

Sergey Sharybin (sergey) requested review of this revision.Nov 26 2021, 6:32 PM

Sergey Sharybin (sergey) created this revision.

Harbormaster completed remote builds in B18992: Diff 45320.Nov 26 2021, 6:32 PM

Noticed there is something wrong with the multi-device rendering.

Fix multi-device case.

Harbormaster completed remote builds in B18993: Diff 45321.Nov 26 2021, 6:49 PM

Alaska (Alaska) awarded a token.Nov 26 2021, 8:54 PM

Raimund Klink (Raimund58) awarded a token.Nov 29 2021, 10:38 AM

Limit the tile size in the ionterface.

Harbormaster completed remote builds in B19054: Diff 45425.Nov 29 2021, 5:44 PM

Raimund Klink (Raimund58) mentioned this in T93350: Cycles renders shows black during rendering huge resolutions.Nov 30 2021, 1:57 AM

Sergey Sharybin (sergey) added a task: T93350: Cycles renders shows black during rendering huge resolutions.Nov 30 2021, 9:24 AM

Jeroen Bakker (jbakker) added a comment.Nov 30 2021, 3:05 PM

This comment was removed by Jeroen Bakker (jbakker).

Seems to work well, tested on macOS and Linux. Only minor comments.

intern/cycles/blender/display_driver.cpp
283	Maybe rename to something like `num_textures_used`. Based on the name my first guess this was some kind of reference counting thing, but it's just a logging thing.
286	An `assert(gl_id == 0)` in the destructor would be good to catch resource leaks.
369	Same comments as for `GLTexture`.
888	Was already in the previous code, but I think this needs `if (use_gl_context_) {`?

This revision now requires changes to proceed.Nov 30 2021, 3:15 PM

Some compilation errors when compiling the patch. They were gone after rebasing with master.
From end user point of view it fixes the issue.
When rendering I did see some glitches depending on the zoom level your in.

This was on a AMD Vega with open source drivers.

system-info.txt12 KBDownload

Sergey Sharybin (sergey) marked 3 inline comments as done.Nov 30 2021, 3:39 PM

Sergey Sharybin (sergey) added inline comments.

intern/cycles/blender/display_driver.cpp
283	`GLTexture::num_textures_used` seems a bit redundant. Propose `GLTexture::num_used` (and similar for the PBO class).
286	Good idea!
888	This is a good catch!

Addressing comments from Brecht.

Looking into artifacts reported by Jeroen now.

Harbormaster completed remote builds in B19102: Diff 45493.Nov 30 2021, 3:40 PM

Clamp texture to edge.

Solves repetition of poixels on the tile border issue which i've managed
to reproduce here.

HOpefully is the same issue as Jeroen was experiencing :)

Harbormaster completed remote builds in B19103: Diff 45495.Nov 30 2021, 3:47 PM

Brecht Van Lommel (brecht) accepted this revision.Nov 30 2021, 4:18 PM

This revision is now accepted and ready to land.Nov 30 2021, 4:18 PM

I'll commit this and handle any bug reports, since Sergey is away.

Closed by commit rB5e37f70307bd: Fix T93350: Cycles renders shows black during rendering huge resolutions (authored by Sergey Sharybin (sergey), committed by Brecht Van Lommel (brecht)). · Explain WhyDec 7 2021, 7:02 PM

This revision was automatically updated to reflect the committed changes.

Brecht Van Lommel (brecht) added a commit: rB5e37f70307bd: Fix T93350: Cycles renders shows black during rendering huge resolutions.

Brecht Van Lommel (brecht) added a reverting change: rB204ae33d7555: Revert "Fix T93350: Cycles renders shows black during rendering huge….Dec 7 2021, 8:52 PM

Reverted in rB204ae33d7555: Revert "Fix T93350: Cycles renders shows black during rendering huge…. This is leading to freezing of the entire desktop for a few seconds when stopping 3D viewport rendering on my Linux / NVIDIA system. I will investigate.

This revision is now accepted and ready to land.Dec 7 2021, 8:52 PM

Brecht Van Lommel (brecht) requested changes to this revision.Dec 7 2021, 8:52 PM

This revision now requires changes to proceed.Dec 7 2021, 8:52 PM

I was trying to reproduce the issue for a while here and could not. Although, think I am on latest drivers, so that might be a factor which explains difference between your and mine setup. From a quick look in the code didn't see anything obviously wrong either.

So lets have some (possibly remote over ssh) debug session when you're here to get some clues about what causes the freeze.

Btw, does it happen on any scene or only on a specific one?

Brecht Van Lommel (brecht) mentioned this in rB3a4952e7c202: Fix Cycles updating display unnecessarily when stopping 3D viewport.Jan 6 2022, 7:16 PM

Fix/workaround issues with NVIDIA driver freeze.

It appears that when a display driver texture update happens on the render thread, then activating the OpenGL context on the main thread hangs for 5 seconds if the command has not yet finished.

The fix is to wait for this update to be finished on the render thread, and only then destroy the render thread and OpenGL resources on the main thread. It's unclear if this is required by the OpenGL spec or a driver issue, but it seems to solve the problem.

Harbormaster completed remote builds in B19814: Diff 46703.Jan 6 2022, 8:14 PM

Brecht Van Lommel (brecht) accepted this revision.Jan 6 2022, 8:24 PM

This revision is now accepted and ready to land.Jan 6 2022, 8:24 PM

Is the issue limited to activating an OpenGL context with non-finished PBO unpack on another thread is the issue? As in, does activating OpenGL context on the same thread while unpacking is still happening on the same thread is all fine? If you can verify a high-res world-only viewport render doesn't hand during rendering that'd be lovely (to ensure much more often texture updates from render threads than draws do not cause freeze).

intern/cycles/blender/display_driver.cpp
878	For the traceability would be nice to state driver version.

In D13385#364372, @Sergey Sharybin (sergey) wrote:

Is the issue limited to activating an OpenGL context with non-finished PBO unpack on another thread is the issue? As in, does activating OpenGL context on the same thread while unpacking is still happening on the same thread is all fine? If you can verify a high-res world-only viewport render doesn't hand during rendering that'd be lovely (to ensure much more often texture updates from render threads than draws do not cause freeze).

I can't get it to hang in other cases like a fast high res render.

It may also be that the issue happens specifically because the render thread gets destroyed.

intern/cycles/blender/display_driver.cpp
878	I'll update the comment.

Closed by commit rBae28d90578be: Fix T93350: Cycles renders shows black during rendering huge resolutions (authored by Brecht Van Lommel (brecht)). · Explain WhyJan 7 2022, 6:37 PM

This revision was automatically updated to reflect the committed changes.

Brecht Van Lommel (brecht) added a commit: rBae28d90578be: Fix T93350: Cycles renders shows black during rendering huge resolutions.

Brecht Van Lommel (brecht) added a commit: rCd3a486d3fb2d: Fix T93350: Cycles renders shows black during rendering huge resolutions.Jan 27 2022, 11:37 PM

Brecht Van Lommel (brecht) mentioned this in rC2e4238c9eeea: Fix Cycles updating display unnecessarily when stopping 3D viewport.

Brecht Van Lommel (brecht) added a commit: rCd29923554a7a: Fix T93350: Cycles renders shows black during rendering huge resolutions.

Bruno (brunochartier) mentioned this in T96133: MacOS desktop freeze and crash on 3.1 beta up to 3.2.2 when moving mesh in Cycles Rendered view (Cycles CPU mode).Mar 5 2022, 12:48 AM

Bruno (brunochartier) added a subscriber: Bruno (brunochartier).Aug 17 2022, 3:47 PM

Fix T93350: Cycles renders shows black during rendering huge resolutions
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 45321

intern/cycles/blender/display_driver.h

intern/cycles/blender/display_driver.cpp

intern/cycles/device/cuda/graphics_interop.cpp

intern/cycles/integrator/path_trace.h

intern/cycles/integrator/path_trace.cpp

intern/cycles/integrator/path_trace_display.h

intern/cycles/integrator/path_trace_display.cpp

intern/cycles/integrator/path_trace_work.cpp

intern/cycles/integrator/path_trace_work_gpu.cpp

intern/cycles/session/display_driver.h

intern/cycles/session/session.cpp

intern/cycles/session/tile.h

intern/cycles/session/tile.cpp

Fix T93350: Cycles renders shows black during rendering huge resolutionsClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 45321

intern/cycles/blender/display_driver.h

intern/cycles/blender/display_driver.cpp

intern/cycles/device/cuda/graphics_interop.cpp

intern/cycles/integrator/path_trace.h

intern/cycles/integrator/path_trace.cpp

intern/cycles/integrator/path_trace_display.h

intern/cycles/integrator/path_trace_display.cpp

intern/cycles/integrator/path_trace_work.cpp

intern/cycles/integrator/path_trace_work_gpu.cpp

intern/cycles/session/display_driver.h

intern/cycles/session/session.cpp

intern/cycles/session/tile.h

intern/cycles/session/tile.cpp

Fix T93350: Cycles renders shows black during rendering huge resolutions
ClosedPublic
Actions

Revision Contents
Changeset List