Page MenuHome

Fix T82591: Performance regression when rendering at very high resolution
Needs ReviewPublic

Authored by Jeroen Bakker (jbakker) on Wed, Nov 18, 10:10 AM.

Details

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
Branch
arcpatch-D9591 (branched from master)
Build Status
Buildable 11360
Build 11360: arc lint + arc unit

Event Timeline

Jeroen Bakker (jbakker) requested review of this revision.Wed, Nov 18, 10:10 AM
Jeroen Bakker (jbakker) created this revision.
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
207

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.Wed, Nov 18, 11:56 AM
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)
Jeroen Bakker (jbakker) marked 2 inline comments as done.
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)
  • Small code review comments
  • Fix spelling in comment
  • Fixed crash during texture painting.
source/blender/blenkernel/intern/image_gpu.c
87

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.

Jeroen Bakker (jbakker) marked an inline comment as done.
  • Removed has_other_size function

Merged 2.91 into master to remove unrelated changes

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
329

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.

333

tile is assumed to be non-NULL here and ok here, it's not clear that is safe.

345

BKE_image_tag_time is for garbage collection, not image sequences. So I'm not sure what this comment is about.

837

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.

Jeroen Bakker (jbakker) marked 3 inline comments as done.
  • Switch from partial refresh to full refresh based on the requested area size

Replaced areas with tiles to improve the performance for many core systems

Jeroen Bakker (jbakker) planned changes to this revision.Fri, Nov 20, 11:51 AM
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)
  • Fixed Comment
  • Migrated requested render slot to use tagging
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Fri, Nov 20, 1:41 PM