Page MenuHome

Cycles-X: Implement viewport navigation resolution divider override
AbandonedPublic

Authored by Alaska (Alaska) on Aug 28 2021, 10:45 AM.
Tags
None
Subscribers
None
Tokens
"Love" token, awarded by gilberto_rodrigues."Like" token, awarded by GeorgiaPacific."Love" token, awarded by Raimund58.

Details

Summary

As per the request of a user on devtalk I have implemented an override for the viewport navigation resolution divider.

The option can be found in the Performance -> Viewport tab of the render properties panel.

Diff Detail

Repository
rB Blender
Branch
cycles-x-override-naviagation-size (branched from master)
Build Status
Buildable 16664
Build 16664: arc lint + arc unit

Event Timeline

Alaska (Alaska) requested review of this revision.Aug 28 2021, 10:45 AM
Alaska (Alaska) created this revision.
Alaska (Alaska) edited the summary of this revision. (Show Details)Aug 28 2021, 10:45 AM
Alaska (Alaska) added a comment.EditedAug 28 2021, 10:51 AM

This is just an initial patch to add the functionality. Refining of design and property names are welcome.

This code is based on rB66c1b23aa10d: Cycles/BI: Add a pixel size option for speeding up viewport rendering and I'm not entirely sure if everything I have added is needed.

Little bits of the code can probably be simplified, but due to my limited knowledge of the Blender source code I do not know which options can be simplified or how.

Edit: In the inline comment below, I meant a "maximum value of 64" rather than minimum.
And the situations where "useless viewports" appear is when you set both the Pixel Size and Start Pixel Size to 8x. That's effectively 64 divisions on the viewport. That turns a 1080x1080 viewport into a 16x16 viewport. Basically useless. So I limited the value to 64 (which is 32 due to the way the code works?) which results in a better result. The main issues this leads to is it limits the number of resolution divisions for people with extremely high resolution displays (E.G 15,360x8640 or higher) in the future.

intern/cycles/integrator/render_scheduler.cpp
127

A minimum value of 64 has been added as letting the value decide itself lead to useless viewports with very small numbers of pixels on my 1920x1080 monitor. There is probably a better approach to this but this is what I came up with.

Alaska (Alaska) edited the summary of this revision. (Show Details)Aug 28 2021, 11:12 AM

Small adjustments and code clean up and adjust resolution limiter

intern/cycles/integrator/render_scheduler.cpp
130–136

This is my new resolution limiter to ensure you always have a "usable" viewport.

Effectively it gives an upper limit to the resolution_divider so that the rendered viewport can't be smaller than min_viewport_resolution (64 pixels) on the largest axis.

If the pixel_size parameter is large enough and the viewport size small enough it can produce values less pixel_size. This is bad because pixel_size should be the minimum value, but its technically fine as it is accounted for with at around line 940 of render_scheduler.cpp:

/* Never higher resolution that the pixel size allows to (which is possible if the scene is
 * simple and compute device is fast). */
const int new_resolution_divider = max(resolution_divider_for_update, pixel_size_);

You may also notice const int max_resolution_divider_width =. I have added these here rather than directly using the equation because next_power_of_two() produces a uint which the causes the max() in state_.resolution_divider = max() to produce compile errors.

Cleanup whitespaces and make small adjustments to resolution limiter

Sergey Sharybin (sergey) requested changes to this revision.Aug 30 2021, 10:16 AM

There are few issues with the option I can think of immediately:

  • It is dependent on both scene and device: something what works for one artist will not work good for another, making it tricky to get best behavior in studio environment.
  • Getting the good value is purely empirical for artists as there is no easy rule to know what to set the value to.

While the options might be needed for some obscure corner case, I'd rather not jump straight into exposing such options and first try to understand why system does not work as expected first. For example, we do attempt on mimmicing the empirical observation to adapt to specific scene/hardware, similar to what artist would do. So, why does this not work good in the example you've linked?
Additionally, we do schedule more samples when resolution divider active, so why this doesn't help either (and makes things worse than in 2.93)?

Those things are to be investigated and made it so Automatic mode works well before adding option and considering the issue closed.

This revision now requires changes to proceed.Aug 30 2021, 10:16 AM
Alaska (Alaska) added a comment.EditedAug 30 2021, 11:18 AM

There are few issues with the option I can think of immediately:
While the options might be needed for some obscure corner case, I'd rather not jump straight into exposing such options and first try to understand why system does not work as expected first. For example, we do attempt on mimmicing the empirical observation to adapt to specific scene/hardware, similar to what artist would do. So, why does this not work good in the example you've linked?

I believe the issue is related to the system being too aggressive with resolution divisions in the users opinion. The system is designed to aim for an update speed of 30 times per second. This is good for navigating the viewport where responsive updates are important, but when you're adjusting finer properties like material properties, or the finer placement of objects where image quality/resolution may be preferred over fast refreshes, the system doesn't work as the user wants it too. Adding an option in the user interface to override the automatic resolution divider lets the user choose, do they want an automatic resolution divider for fast performance while navigating the scene? Or a higher quality image while adjusting finer details? In theory a automatic system can be setup for detecting situations like this. If the camera isn't moving then maybe the resolution divider is more focused on image quality than performance? Not sure.

Additionally, we do schedule more samples when resolution divider active, so why this doesn't help either (and makes things worse than in 2.93)?

Similar to above, I believe this does not help because even with the sample count being higher, the fact the resolution is lower is the issue. With such a low resolution it's hard to make out fine details and thus manually overriding the resolution divider can help.

Another thing I would like to suggest is my own personal reason for wanting this feature implemented.
When showing a rendered viewport scene to someone, seeing the viewport reset to a lower resolution then increase to a higher resolution again when I make subtle changes is distracting for the person I'm showing the scene too. To remove this distraction I can manually override the resolution divider to always start at Pixel Size, or maybe half of pixel size so the transition from a lower resolution to a higher resolution is less distracting. And it's not just a distracting thing for the person, I also find it distracting sometimes.

However, I will be guided by you and the rest of the Cycles development team on what you want to do here.

Whether it is a target FPS at "fault" should be easy to verify by modifying RenderScheduler::guess_viewport_navigation_update_interval_in_seconds().

However, it is still not very clear to me why Update 2 is so much more noisy even though more samples are expected to be scheduled. There is a bit questionable if (resolution_divider <= pixel_size_ * 2) {block in RenderScheduler::get_num_samples_during_navigation(). But that would not explain why manual clamp of pixel size improves quality for you.

Some things (combination of them, perhaps) to try to make Automatic mode work better and hopefully eliminate the need of manual control:

  • Limit divider to some derivative of the target pixel size (i.e. never go above 4 or 8 of the pixel sizes)
  • Lower the navigation speed if the system can't keep up with the non-blocky pixel size (not sure if some explicit code for this is needed, but ideally simple scenes will be rendered, say, at 1/4 of resolution at 30fps, and more complex ones will render at a lower speedrate during navigation).
  • Check whether multi-sample scheduling works as expected.

It would help to have file and configuration used to reproduce the worse-than-2.93 scenario.

Alaska (Alaska) added a comment.EditedAug 30 2021, 11:55 AM
  • Check whether multi-sample scheduling works as expected.

What is the expected behavior? From some preliminary tests 4 samples is rendered as you navigate (8x resolution division), then the resolution divider drops a step (4x resolution division) and it renders 4 samples, then it drops another step (2x resolution division) and renders 1 sample, then drops the last step to full resolution and starts rendering from 1 sample again.

This appears to be right based on how I've interpreted the code.

This test was: Take a complex scene like flat-archivis from Blender demo website then render the scene with a low number of CPU threads.

Alaska (Alaska) added a comment.EditedAug 30 2021, 12:27 PM

However, it is still not very clear to me why Update 2 is so much more noisy even though more samples are expected to be scheduled. There is a bit questionable if (resolution_divider <= pixel_size_ * 2) {block in RenderScheduler::get_num_samples_during_navigation(). But that would not explain why manual clamp of pixel size improves quality for you.

if (resolution_divider <= pixel_size_ * 2) { is most probably the fault with regard to Update 2 looking noisy. Looking at the image it appears the Update 2 is when resolution_divider = pixel_size_ * 2 and thus that part of the code path is used. As such Update 2 is rendered at 1 sample while Update 1 is rendered at 4 samples making it appear more noisy.

But that would not explain why manual clamp of pixel size improves quality for you.

It does not improve the sample count, it simply improves the resolution (assuming I pick a higher resolution than auto). I personally perceive this higher resolution as an improvement to quality when compared to higher samples sample count in these types of situation. But this is personal opinion.

Some things (combination of them, perhaps) to try to make Automatic mode work better and hopefully eliminate the need of manual control:

  • Limit divider to some derivative of the target pixel size (i.e. never go above 4 or 8 of the pixel sizes)
  • Lower the navigation speed if the system can't keep up with the non-blocky pixel size (not sure if some explicit code for this is needed, but ideally simple scenes will be rendered, say, at 1/4 of resolution at 30fps, and more complex ones will render at a lower speedrate during navigation).

These should be a easy change to make. With Cycles-X as the base, change state_.resolution_divider = start_resolution_divider_ * 2; on line 121 of render_scheduler.cpp to state_.resolution_divider = min(start_resolution_divider_ * 2, 8); (The 8 acts as a resolution divider of 4) and the code already in place should take care of the "more complex scenes will render at a lower speedrate during navigation"

  • Check whether multi-sample scheduling works as expected.

Check my previous comment.

What is the expected behavior?

You've described an expected behavior of the code.

Interesting that the Update 2 in the post about Cycles X is still so noisy though.

state_.resolution_divider = min(start_resolution_divider_ * 2, 8)

Should be based on pixel size, so something like state_.resolution_divider = min(start_resolution_divider_ * 2, 8 * pixel_size_);

Should be based on pixel size, so something like state_.resolution_divider = min(start_resolution_divider_ * 2, 8 * pixel_size_);

Upon testing it seems a limit already exists. The resolution divider already seems unable to exceed 8 divisions. If we wish to change the limit to 4 divisions, then the change should probably be made where this limit is already implemented rather than add it here. I will look into this later.

Upon testing it seems a limit already exists. The resolution divider already seems unable to exceed 8 divisions. If we wish to change the limit to 4 divisions, then the change should probably be made where this limit is already implemented rather than add it here. I will look into this later.

The limit is already in the code at around line 940 in render_schedule.cpp

/* Don't let resolution to go below the desired one: better be slower than provide a fully
 * unreadable viewport render. */
start_resolution_divider_ = min(new_resolution_divider, default_start_resolution_divider_);
Line 33: default_start_resolution_divider_(pixel_size * 8)

My new resolution limiter or something similar to it could be useful here:

const int min_viewport_resolution = 64;
if (max(buffer_params.width / state_.resolution_divider,
   buffer_params.height / state_.resolution_divider) < min_viewport_resolution / 2) {
  const int max_resolution_divider_width = next_power_of_two(buffer_params.width/min_viewport_resolution);
  const int max_resolution_divider_height = next_power_of_two(buffer_params.height/min_viewport_resolution);
  state_.resolution_divider = max(max_resolution_divider_width, max_resolution_divider_height);
}

As it stops the viewport from going below 64 pixels on the largest axis, which can be achieved if the viewport is small and/or the pixel size is large and the scene is slow to render.

Alaska (Alaska) added a comment.EditedSep 1 2021, 5:21 AM

@Sergey Sharybin (sergey) I have done a collection of tests relating to changing values within the automatic resolution divider to make the system better and to reduce the need for this patch. Here are my results.

Tests I ran and thoughts I came up with:


Test 1:
If the initial resolution divider is 4, replace it with 2.

Reason for making this change:
A resolution divider of 4 with 4 samples per pixel is almost computationally identical to a resolution divider of 2 at 1 sample per pixel (the current behaviour). I personally believe that a higher resolution is more important than a higher sample count in this specific situation.

Positives:
The resolution is higher in scenes that used to render in that range which is good in my opinion

Negatives:
As soon as the scene becomes complex enough (E.G. Move to a slightly more computationally expensive scene), the resolution divider jumps straight from 2 to 8. This change is "quality" is distracting.

Conclusion:
I do not believe this is a valid candidate for Cycles-X. Instead something like D12367: Cycles-X: Fix incorrect performance calculation at certain navigation resolutions should help in resolving the issue this change is trying to address without as many negative effects.


Test 2:
Change sample count during navigation to 2 samples per pixel for all resolution dividers greater than 2.

Reason for making this change:
By lowering the sample count, performance will be higher meaning the automatic system is more likely to favor a higher resolution.

Positives:
The resolution tended to be higher than before the change.

Negatives:
It is worse than before in some cases. There are situations where the resolution divider is identical between this change and no change. And in those situations all you're experiencing is a negative side effect, the sample count is lower.

Conclusion:
I'm unsure about this change. It has negatives and positives.


Test 3:
Change sample count during navigation to 1 sample per pixel at all times

Reason for making this change:
It fixes some issues where the resolution divider does not want to drop from 4 to 2, and it guarantees a higher resolution is used in most situations.

Positives:
A higher resolution is used in most cases. The issue with the resolution divider not changing between 4 and 2 is also fixed with this (Can also be fixed with something like D12367: Cycles-X: Fix incorrect performance calculation at certain navigation resolutions)

Negatives:
The sample count is lower in most situations and as such the viewport becomes un-usable in cases that the scene was having a really hard time rendering on your system before this change.

Conclusion:
I do not believe this is a valid candidate for Cycles-X. The drop in sample count is too much of a sacrifice at resolution dividers of 4 and 8.


Test 4:
Decrease the target frame rate during navigation to 10fps

Reason for making this change:
By decreasing the target frame rate during navigation we allow the viewport to render at a higher resolution.

Positives:
Generally renders the scene at a higher resolution.

Negatives:
Frame rate is lower in many situations

Conclusion:
I do not believe this is a valid candidate for Cycles-X. The drop in frame rate is too much of a sacrifice to force onto everyone with the automatic system.


I still believe this patch (D12338) can still be useful. However, there still potentially smaller changes that could be made to the automatic system that further improves it that I haven't thought of testing yet.

Rebase on latest Cycles-X and remove resolution divider limiter

Rebase on latest master

Alaska (Alaska) updated this revision to Diff 46097.EditedDec 16 2021, 1:24 AM
  • Rebase on latest master - Some of the code probably isn't needed, but I don't know enough about the Blender source code to figure out what can be removed.
Alaska (Alaska) abandoned this revision.Nov 21 2022, 3:12 AM