Workbench: Fix: Performance Regressions #113251

Merged

Miguel Pozo merged 8 commits from pragma37/blender:pull-workbench-opti-cleanup into blender-v4.0-release

2023-10-06 15:31:53 +02:00

Miguel Pozo commented

2023-10-04 18:08:54 +02:00

Member

Optimize Workbench performance so it's on par with the previous implementation.
Most of these changes are barely noticeable on powerful GPUs,
but can cause a notable performance improvement on old or low-end hardware.

Avoid unnecessary texture copies and draw directly to the viewport textures.
Optimize-out depth/stencil reads, using stencil testing instead.
Avoid using Texture::clear and use framebuffer clears instead.
Avoid framebuffer state changes (always use the same attachments).
Avoid constant variation of acquired TextureFromPools.

Optimize Workbench performance so it's on par with the previous implementation. Most of these changes are barely noticeable on powerful GPUs, but can cause a notable performance improvement on old or low-end hardware. * Avoid unnecessary texture copies and draw directly to the viewport textures. * Optimize-out depth/stencil reads, using stencil testing instead. * Avoid using `Texture::clear` and use framebuffer clears instead. * Avoid framebuffer state changes (always use the same attachments). * Avoid constant variation of acquired `TextureFromPool`s. Fix #113010

Miguel Pozo added this to the 4.0 milestone 2023-10-04 18:08:54 +02:00

Miguel Pozo added the

EEVEE & Viewport

label 2023-10-04 18:08:54 +02:00

Miguel Pozo added 7 commits 2023-10-04 18:09:10 +02:00

840ad5712c Avoid unnecesary texture copies and framebuffer changes

3a74b4576e Add shadow variation to composite shader

8b501eaf20 Remove unused WORKBENCH_OUTLINE

469aa4c0ad Replace depth/stencil reads with stencil ops

8e4836fef3 Allocate depth_in_front for image renders

d92e1bb4a3 FIx depth_tx feedback loop warning

buildbot/vexp-code-patch-coordinator Build done. Details

41ec8d6d28 Merge branch 'blender-v4.0-release' into pull-workbench-opti-cleanup

Miguel Pozo changed title from ~~Workbench: Fix: Performance Regressions~~ to Workbench: Fix: Performance Regressions

2023-10-04 18:10:05 +02:00

pragma37 changed target branch from main to blender-v4.0-release

2023-10-04 18:10:07 +02:00

Miguel Pozo added the

label 2023-10-04 18:10:32 +02:00

Miguel Pozo requested review from Clément Foucault 2023-10-04 18:10:46 +02:00

Miguel Pozo commented

2023-10-04 18:11:11 +02:00

Author

Member

@blender-bot package

@blender-bot package

Blender Bot commented

2023-10-04 18:11:13 +02:00

Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR113251) when ready.

Miguel Pozo referenced this pull request

2023-10-04 18:14:44 +02:00

Regression: Blender 4.0 3D View resizing in Workbench mode slower and laggier than 3.6 #113010

Clément Foucault requested changes 2023-10-05 17:45:39 +02:00

Clément Foucault left a comment

Member

Overall seems good. Just have 2 questions. See my comments.

Overall seems good. Just have 2 questions. See my comments.

source/blender/draw/engines/workbench/workbench_effect_antialiasing.cc

						
				@ -301,3 +310,1 @@

				    GPU_texture_copy(depth_tx, sample0_depth_tx_);

				    /* There's no depth_in_front_tx in scene image renders. */

				  }

				  /** Always acquire to avoid constant allocation/deallocation. */

Clément Foucault commented

2023-10-05 17:37:08 +02:00

Member

I don't understand this. The use of temp texture should not make them recreated every frame.

I don't understand this. The use of temp texture should not make them recreated every frame.

Miguel Pozo commented

2023-10-05 18:34:47 +02:00

Author

Member

If you are using TAAx16, for example, the engine is going to acquire smaa_edge_tx_ for the first 5 samples, but not for the rest.
So, in sample 13, the texture is going to be released from the pool, and the next frame will be recreated again.

If you are using TAAx16, for example, the engine is going to acquire `smaa_edge_tx_` for the first 5 samples, but not for the rest. So, in sample 13, the texture is going to be released from the pool, and the next frame will be recreated again.

Clément Foucault commented

2023-10-05 18:52:51 +02:00

Member

Does that manifest by small itches on low end systems?

Does that manifest by small itches on low end systems?

Miguel Pozo commented

2023-10-05 19:39:44 +02:00

Author

Member

It's hard to measure, but I'm pretty sure it won't help.
Another reason for doing this is to ensure Framebuffers don't change their attachments from sample to sample (since they will if the order of acquired textures changes), which can indeed cause performance issues.
Probably not in this case, at least right now, since they're the last acquired textures.

But I think it would be good practice to ensure engines always request the same textures in the same order unless there's a feature enabled/disabled or some other kind of "long-term" change.

It's hard to measure, but I'm pretty sure it won't help. Another reason for doing this is to ensure Framebuffers don't change their attachments from sample to sample (since they will if the order of acquired textures changes), which can indeed cause performance issues. Probably not in this case, at least right now, since they're the last acquired textures. But I think it would be good practice to ensure engines always request the same textures in the same order unless there's a feature enabled/disabled or some other kind of "long-term" change.

Clément Foucault commented

2023-10-06 01:36:04 +02:00

Member

I think it is wrong to fix the issue at the user level when it's bad API implementation. So maybe we should look into some way to make sure a call to draw::Texture::acquire() will always return in the same texture being acquired. But that's unrelated to this patch.

I think it is wrong to fix the issue at the user level when it's bad API implementation. So maybe we should look into some way to make sure a call to `draw::Texture::acquire()` will always return in the same texture being acquired. But that's unrelated to this patch.

source/blender/draw/engines/workbench/workbench_private.hh Outdated

						
				@ -243,11 +250,16 @@ class MeshPass : public PassMain {

				                             ImageUser *iuser = nullptr);

				};

				struct StencilBits {

Clément Foucault commented

2023-10-05 17:25:51 +02:00

Member

Why not use an enum here? Also I think members should be snake case.

Why not use an `enum` here? Also I think members should be snake case.

Miguel Pozo commented

2023-10-05 18:42:19 +02:00

Author

Member

To ensure the values are uint8_t.
I could enum StencilBits: unsigned char instead, but then you leak into the parent namespace.
Or use an enum class, but then you need custom operators (I know there's a macro for that, but still...).

To ensure the values are `uint8_t`. I could `enum StencilBits: unsigned char` instead, but then you leak into the parent namespace. Or use an enum class, but then you need custom operators (I know there's a macro for that, but still...).

Clément Foucault commented

2023-10-05 18:51:31 +02:00

Member

Yes use enum class StencilBits : uint8_t and use uint8_t(StencilBits::OBJECT_IN_FRONT) for conversion.

Yes use `enum class StencilBits : uint8_t` and use `uint8_t(StencilBits::OBJECT_IN_FRONT)` for conversion.

pragma37 marked this conversation as resolved

Miguel Pozo added 1 commit 2023-10-05 20:14:32 +02:00

0e934a263e Convert StencilBits to enum class

Clément Foucault approved these changes 2023-10-06 14:09:33 +02:00

Miguel Pozo referenced this issue from a commit

2023-10-06 15:31:53 +02:00

Workbench: Fix: Performance Regressions

Miguel Pozo merged commit fe39456ba5 into blender-v4.0-release

2023-10-06 15:31:53 +02:00

Miguel Pozo deleted branch pull-workbench-opti-cleanup

2023-10-06 15:31:57 +02:00

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

3 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#113251

No description provided.