Page MenuHome

T53971: Render Result showing the wrong renderlayer in image editor
ClosedPublic

Authored by Cheryl Chen (chnchryl) on Mar 9 2018, 10:57 PM.

Diff Detail

Repository
rB Blender

Event Timeline

Brecht Van Lommel (brecht) requested changes to this revision.Mar 11 2018, 11:07 PM

Thanks for the patch, some comments.

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

We shouldn't be including internal headers from the render/ module.

If you need to access something in the Render struct, add a function like RE_HasSingleLayer(struct Render *re) to RE_pipeline.h and pipeline.c.

623–634

I think the simpler solution could be to just not restore the original layer in this case, since the active layer will already have been actived during the render already. Any reason that doesn't work?

626

Code style convention:

if (re->r.scemode & R_SINGLE_LAYER) {
636

Code style convention:

}
else {
    ...
}
This revision now requires changes to proceed.Mar 11 2018, 11:07 PM

Thanks for the feedback!

I agree that not restoring the original layer will be more straightforward. However, I'm having some trouble pinpointing which factors affect the displayed render layer so that I can prevent it from switching. I think the final render result is determined by ‘image_get_render_result()’, but I can’t find which part affects the displayed tiles while rendering.

I think it would be helpful if ‘iuser.layer’ is still updated to the proper active layer index, since it affects what shows up in the ‘layer’ button. It would be best if I could directly set it to the rendered image’s index, but the indexes of the render layers and the rendered result aren’t the same. I also tried getting the active layer, but sometimes it gives me the wrong one... (probably because of the layer index issue?)

render_image_update_pass_and_layer() already sets sima->iuser.layer to the layer currently being rendered in the image editor, If we do nothing in render_image_restore_layer() in case of single layer rendering, isn't that enough, so that we don't have to worry at all about figuring out what the active layer is?

It gets a bit complicated in general with the sequencer, compositing, operator properties, .. best to avoid all that if possible.

Ah...right. I did try doing nothing in render_image_restore_layer(), but for some reason, sima->iuser.layer is not always the right value before all render layers are rendered. I think it is because rr->renlay->name, which is used to look up the layer index, sometimes gets set to a previous layer's name (though somehow, the layer name being looked up is correct while rendering). Also, currently sima->iuser.layer is never set to 0, since it is only set when layer != rj->last_layer, and rj->last_layer is always 0...

Anyway, I'll poke around some more and see if I can avoid searching the layer again.

I think the issue was with render_get_active_layer() looking up re->r.actlay in rr->layers (maybe as an optimisation?). Since rr->layers only contains the rendered layers, it sometimes returns the wrong layer when not layers are rendered. Also sima->iuser.layer did not always get updated because of the layer != rj->last_layer condition...

Thanks, I'll commit this with some additional fixes.

source/blender/render/intern/source/pipeline.c
271

This isn't correct since re->r.layers is a list of SceneRenderLayer so that can't be used as a RenderLayer.

This revision was not accepted when it landed; it landed in state Needs Review.Mar 16 2018, 1:03 AM
This revision was automatically updated to reflect the committed changes.