Page MenuHome

Cycles/Render API: Implement "Set Render focus" feature
Needs RevisionPublic

Authored by Lukas Stockner (lukasstockner97) on Jan 12 2016, 12:45 AM.
Tags
None
Tokens
"Like" token, awarded by Saiman."Like" token, awarded by Lapineige."Yellow Medal" token, awarded by aliasguru."Love" token, awarded by dingto.

Details

Summary

This patch adds "Set Render focus" to the RenderEngine API and implements it in Cycles.
Generally, the task of the function is to allow users to specify areas that they want the renderer to focus on
by clicking into the rendering image with Ctrl held down. This is a bit vague on purpose to allow different engines
to implement it as it is suitable for them.

The Cycles implementation reacts to this function by reordering the tiles so that the origin of the Circle and
Hilbert Spiral patterns now is at the clicked position. This allows users to specify which areas should be processed
next for a quicker preview and to allow to check specific off-center areas before the rendering would normally reach them.

The UI side is loosely based on T37220.
ToDo: BI support!

Also includes (would be committed separately, but not worth opening a second diff):

Cycles: Move Hilbert Spiral generation to separate function, deduplicate tile generation

The Hilbert Spiral code is now separated from the tile generation code, which allows to
handle all tile orders in a single loop and therefore deduplicate some code.

Diff Detail

Repository
rB Blender
Branch
click_center

Event Timeline

Here's an example image (stopped after some time):


Also, I just noticed that weird things sometimes happen to the brightness of the image, probably due to some threading conflict when accessing the samples variable that the buffer uses. I'll have a look tomorrow.
Another addition that would make this more useful would be to have a kind of prepass that samples everything at 10% of the samples before fully refining the rest of the image - that might even be a replacement for the progressive refine mode.

Sergey Sharybin (sergey) requested changes to this revision.Jan 12 2016, 10:02 AM
Sergey Sharybin (sergey) edited edge metadata.

Initial review.

Also, focus is a bad name, conflicts with camera focus and sounds as re-focus from CV area (was my first idea actually and I thought "WOOOW").

Maybe call it " point of interest" or just a "render center" ?

intern/cycles/blender/addon/__init__.py
92

Don't do this, assume Cycles version matches the addon's one.

intern/cycles/blender/blender_session.h
60

Any reason to not use float2 here?

intern/cycles/render/session.h
143

Thats where float2 can be used, matching tile api.

intern/cycles/render/tile.cpp
296

Seems to lack a lock here, causing conflict with tule acquisition.

source/blender/editors/render/render_internal.c
771

Kinda weak. Use modal keymap instead.

This revision now requires changes to proceed.Jan 12 2016, 10:02 AM

Yes, the name might be confusing - maybe calling it something like "interact" and passing the keyboard state (ctrl, shift, alt) would also work, but that conflicts with the image sampling that currently happens when clicking or dragging on the image...

intern/cycles/blender/blender_session.h
60

Not really, I just wasn't sure whether it's fine to use floatx types here.

intern/cycles/render/tile.cpp
296

A lock on the tile_mutex is acquired in the Session code before calling set_center, so that shouldn't be a problem.

source/blender/editors/render/render_internal.c
771

OK, I'll have another look into the UI stuff.
TBH, I had enough after 3h of reading WM and event handling code and just coded it like this to get the feature running :)

Interact is actually an interesting idea to make Game Engine API to work, but the idea of passing keyboard state is wrong. _If_ we go that direction, it should be notifier/event based system, but even making it to work properly in a clean way is quite a project on it's own -- you'll have to design a system to pass stuff via Python API easier, avoiding hardcoded values all over the place and so. And last but not least -- you'll need to re-implement keymap handling on the other side of API.

Now, Cycles is a render engine and should stay like that. Meaning, it's only doing what the actual application is asking it to do, in this case leaving all the event and keymap handling to Blender.

Adam T W (Thordwilk) added a comment.EditedJan 20 2016, 5:50 PM

l would love to have this when working on fur and hair. Just being able to specify those niggling spots on a render instead of having to render a whole chunk of the head would save me a boat load of time when reworking a model.

Wow. That's what I was waiting for and wrote about Sergey Sharybin. How soon it will be release?

Guys, this a code review area for the developers to collaborate. I understand your excitement, but it also complicates following the actual review process.

Lukas, on the second thought, why its a part of render operator? We can have a dedicated operator for that (similar to set white/black point, for the eaae of copy-paste :). Benefits:

  • Easier for you not to deal with modal keymap etc
  • Still has configurable shortcut
  • Has potentioal of being used before the render to change "focus" (still think we should avoid calling feature like this)
  • Should avoid your context area tricks you're doing.