Details

Reviewers

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

Maniphest Tasks

T82591: Performance regression when rendering at very high resolution

Commits

rB4f9e21bdc977: Fix T82591: Performance regression when rendering at very high resolution

Summary

This patch introduces a partial update of GPUTextures of Images. When rendering a large image the GPUTexture could have been scaled. The old implementation would rescale the image and create a new GPUTexture. This solution would only scale and upload the part that has been changed.

The check for requested render slot isn't working in this scenario so it was replaced by correct IMA_GPU_REFRESH tagging.

Performance

Test situation: Default Cube, 4 samples, 19200x10800 tile size 512.

Blender 2.83.9: 4m27s.
This patch: 1m01s.

Patch still needs to be cleaned:

the interface of image_buffer_calc_imbuf_rect is a bit confusing as it was extracted from image_buffer_rect_update without looking to much into its use.

There is still room for more optimizations:

Reduce the time that an image is locked.
- Use task scheduling to update the tiles of an image.
- Generic optimization of the ImBuf scale method.

Diff Detail

Repository: rB Blender

Event Timeline

Jeroen Bakker (jbakker) requested review of this revision.Nov 18 2020, 10:10 AM

Jeroen Bakker (jbakker) created this revision.

Harbormaster completed remote builds in B11319: Diff 31177.Nov 18 2020, 10:10 AM

Clément Foucault (fclem) added a subscriber: Clément Foucault (fclem).Nov 18 2020, 10:48 AM

Clément Foucault (fclem) added inline comments.

source/blender/blenkernel/BKE_image.h
385	nitpick: `BKE_image_update_gputexture_delayed` to be consistent with `BKE_image_update_gputexture`
source/blender/editors/render/render_internal.c
208	typo; `it so` instead of `is to`

Alignment of tiles to GPU texture

Jeroen Bakker (jbakker) retitled this revision from [WIP] Fix T82591: Performance regression when rendering at very high resolution to Fix T82591: Performance regression when rendering at very high resolution.Nov 18 2020, 11:56 AM

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

Jeroen Bakker (jbakker) added reviewers: Brecht Van Lommel (brecht), Clément Foucault (fclem).

Jeroen Bakker (jbakker) added projects: BF Blender (2.91), EEVEE & Viewport, Render & Cycles.

Harbormaster completed remote builds in B11321: Diff 31181.Nov 18 2020, 11:56 AM

Small code review comments

Harbormaster completed remote builds in B11322: Diff 31182.Nov 18 2020, 12:00 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Nov 18 2020, 12:06 PM

Fix spelling in comment

Harbormaster completed remote builds in B11323: Diff 31183.Nov 18 2020, 12:11 PM

Fixed crash during texture painting.

Harbormaster completed remote builds in B11325: Diff 31186.Nov 18 2020, 1:47 PM

Clément Foucault (fclem) added inline comments.Nov 18 2020, 1:52 PM

source/blender/blenkernel/intern/image_gpu.c
86	Nitpick: Maybe `size_match()` is better function name (inverted obviously). But to be fair, having a function that is used only once and is a one liner seems a bit silly and make the code a bit harder to read.

Removed has_other_size function

Harbormaster completed remote builds in B11326: Diff 31187.Nov 18 2020, 2:24 PM

Merged 2.91 into master to remove unrelated changes

Harbormaster completed remote builds in B11327: Diff 31188.Nov 18 2020, 2:27 PM

I see missing tiles with the following steps

Select Cycles as engine and render
Press J to switch to another slot while the render is in progress
Press J to switch back when the render is done

source/blender/blenkernel/intern/image_gpu.c
328	If `ibuf` is `NULL` and partial refresh is requested, I think it should do a full free of the image. I'm not sure if that happens in practice, but seems safer.
332	`tile` is assumed to be non-NULL here and ok here, it's not clear that is safe.
344	`BKE_image_tag_time` is for garbage collection, not image sequences. So I'm not sure what this comment is about.
840	This has O(n^2) time complexity, but I guess it's not that since n is only the number of threads in practice. If it was for all tiles in the image it would be a bigger problem, see rB8c113eb0c475: Render: Use GHash for storing render parts.