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.
Differential D12338
Cycles-X: Implement viewport navigation resolution divider override Authored by Alaska (Alaska) on Aug 28 2021, 10:45 AM. Tags None Subscribers None Tokens
Details 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
Event TimelineComment Actions 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.
Comment Actions 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? Those things are to be investigated and made it so Automatic mode works well before adding option and considering the issue closed. Comment Actions 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.
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. However, I will be guided by you and the rest of the Cycles development team on what you want to do here. Comment Actions 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:
It would help to have file and configuration used to reproduce the worse-than-2.93 scenario. Comment Actions 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. Comment Actions 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.
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.
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 my previous comment. Comment Actions
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.
Should be based on pixel size, so something like state_.resolution_divider = min(start_resolution_divider_ * 2, 8 * pixel_size_); Comment Actions 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. Comment Actions 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. Comment Actions @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: Reason for making this change: Positives: Negatives: Conclusion: Test 2: Reason for making this change: Positives: Negatives: Conclusion: Test 3: Reason for making this change: Positives: Negatives: Conclusion: Test 4: Reason for making this change: Positives: Negatives: Conclusion: 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. Comment Actions
| ||||||||||||||||