Page MenuHome

Remove the auto-disable renderborder feature
ClosedPublic

Authored by Campbell Barton (campbellbarton) on Aug 6 2014, 2:57 AM.

Details

Summary

This removes the feature which disables renderborder when selecting a border region larger than the camera while in camera view. This feature is rather unfriendly now that we have viewport render, as it takes several clicks to define a renderborder the same size as the camera.

It was decided to be removed a while ago, but seems to have been forgotten.

Old behavior: Selecting a renderborder enclosing the entire camera disabled render border.

New behavior: Selecting a renderborder enclosing the entire camera does not disable renderborder, it instead sets the renderborder to the camera. Selecting a renderborder completely outside the camera area (not intersecting the camera area at all), will disable render border. Renderborders the same size as the camera will allow full sample.

Diff Detail

Repository
rB Blender

Event Timeline

Ellwood Zwovic (gandalf3) retitled this revision from to Remove the auto-disable renderborder feature.
Ellwood Zwovic (gandalf3) updated this object.

Now selecting a renderborder completely outside of the camera will disable renderborder. Also updated comment in code.

Could you maybe update the patch and remove all white-space fixes?

Removed whitespace nonsense as requested by @Martijn Berger (juicyfruit)

Sergey Sharybin (sergey) requested changes to this revision.Aug 27 2014, 11:46 AM
Sergey Sharybin (sergey) edited edge metadata.

It needs some usability feedback from @Campbell Barton (campbellbarton) and @Thomas Dinges (dingto), i'm not dealing with the UI.

But the code is wrong -- from the render point of view disabled border and border of the full frame are the totally different cases. If you want this functionallity to work, you are at least modify renderpipeline.c to not set R_BORDER to re->mode when the border equals to the full frame. With the current patch you'll have FSAA and Save Buffers disabled unless you specificly disabel Border before doing final render.

This revision now requires changes to proceed.Aug 27 2014, 11:46 AM

Even if I agree that cycles introduces an use-case for this feature to be reverted I want to point you to the Amaranth addon which has an option to set border to full camera in the W menu, pretty cool!

@Sergey Sharybin (sergey) Thanks for having a look :) I've still quite unfamiliar with all this, so I might take a little while to figure out how to get the border coordinates in the right files.
Incidentally, I couldn't find anything called renderpipeline.c..

My bad, it's pipeline.c in the render folder.

Ellwood Zwovic (gandalf3) edited edge metadata.

Updated to (I hope) not disable full sample when the border is equal to the full frame.

Ellwood Zwovic (gandalf3) updated this object.
Ellwood Zwovic (gandalf3) edited edge metadata.

I think my brain was broken when I submitted the previous revision..

Now it should do everything the previous one claimed to do, except properly :P

Went to update the patch but its not applying to master for me.

@Campbell Barton (campbellbarton) I've made a new patch which seems to apply better (though it looks the same as the old one to me..), but this task is no longer in the "update existing revision" choices.. And before I realized that, I accidentally updated D754 with it instead :(

How do I revert the change to D754, and then upload the patch to this revision?

Disabling border select when drawing a box outside the camera view seems fine.

As for the rest, I don't really like having to check exact render border values all over in the code.
Also dont really like this special handling of FSA.

Couldn't we have view-port render not render outside camera bounds by default? - or only do so when Passeparout is disabled?


@Ellwood Zwovic (gandalf3), dont know how to revert changes to patches, or if its possible, just update the patch to correct it.

@Campbell Barton (campbellbarton), i don't really think disabling render outside of the camera bounds in viewport is always a good idea, in some cases it's rather useful to have full viewport render.

I'm not really sure what is the Passeparout is. It could be a separate option for this in the viewport settings (could be default ON, but should be away to disable). But then you'll need to make it implemented in each of the render engines. For us it'll mean adding a code to deal with Cycles and BI renderers.

Sergey Sharybin (sergey) requested changes to this revision.Sep 5 2014, 9:36 AM
Sergey Sharybin (sergey) edited edge metadata.
This revision now requires changes to proceed.Sep 5 2014, 9:36 AM

@Sergey Sharybin (sergey) - Passeparout is the dark alpha overlay outside the camera view,
it does seem a bit like overloading its meaning to change render behavior based on this, but it also makes some sense - if you disable it you can see the render outside the view-port.

We can always add yet another option, but trying to avoid that... in that case the render border patch might be better.

I'm a bit unclear on what needs to be done now..

As I understand it so far:

  • Need better way to check border size for settings which don't like border render, like full sample. Any ideas on this would be welcome ;)
  • Possibly disable viewport render outside of camera view when passeparout is enabled, or is fully opaque? (Seems to me this isn't really related to border render.. Perhaps should be a separate thing?)

@Campbell Barton (campbellbarton) The idea of disabling rendering outside of camera when using passpartout is genius, my vote's on that and leave the current border behavior alone :D

I for one change the border a lot to focus on different details inside the camera (and occasionally outside the camera, but not nearly as often), then switch back to full camera border to see how my changes look with the rest of the scene.

My original intent in making this patch was to make a quick way to set only the camera to be rendered in the viewport, without having to navigate through panels/select various objects.
Not sure this can be done with the passepartout idea, but maybe I look outside the camera less than I think I do..

IMO using passepartout is redundant, as with the proposed border behavior It's a quick click+swipe to set the rendered area to pretty much anywhere. With passepartout, there is another setting which is limiting the rendered area, even though it doesn't add any more control.. Unless I'm missing something here.

I'd like to see some more user opinions, to see if the passepartout idea is ideal, or if there are other ideas floating around..

Passepartout is also used with the game engine to restrict what you see during gameplay to what the camera sees (as it does not make any use of border rendering).

More often then not, I would want the rendered view to be restricted like this when viewing the scene through the camera, the border rendering can still work with it in its current form.

@Campbell Barton (campbellbarton), that's rather counterintuitive but works. Just one thing, how you're gonna to deal with passeparout with alpha of zero?

P.S. If that option belong to viewport instead of camera that'd be so much more clear..

@Sergey Sharybin (sergey), don't think is _that_ counter intuitive, rationale is:

I want to obscure the region outside the cameras frame - even partially, therefor, theres not much point to render that region)

it makes some sense at least :)

As for zero alpha, I don't have a strong opinion, but think it would be OK to ignore that case and keep it simple - (passepartout == only-render camera-frame).

re, making this viewport... not sure about that, it means if you have a few screens you have to set this option for all of them, instead of just setting the active-camera.

@Campbell Barton (campbellbarton) I just thought of another possible solution:

a. In case user border selects the entire camera frame: do not disable border, just add border rendering for the entire frame
b. In case user selects border outside the camera: disable border render

@Daniel Salazar (zanqdo)

That's already how it works with the current patch..

So can someone commit this patch then? This auto-disable feature is counter productive.

Would it really be that bad if the patch was committed in its initial form?

If the concern is that having a full-camera border disables some BI settings, I'm not sure it's any worse than what happens currently.

My own current workflow (draw border around camera, then re-enable renderborder via the checkbox) takes longer and still results in the same problematic state (full camera renderborder). As such, I already have made it a habit to disable renderborder before doing a final render.

Is this just me? Or are others doing the same thing?

@Ellwood Zwovic (gandalf3) yes exactly, selecting the entire camera space and going to render settings to re-enable border feels rather horrible, let alone explaining that to a student struggling with the rest of Blender :)

I think the behavior change is good, and after D2080 this patch can be even simpler since all the changes outside of view3d_edit.c will become unnecessary.

What we should do though is disable the unnecessary render result copy if the border render is not really a border render. That can be done like this:

diff --git a/source/blender/render/intern/source/pipeline.c b/source/blender/render/intern/source/pipeline.c
index 6c5cc29..6b910dc 100644
--- a/source/blender/render/intern/source/pipeline.c
+++ b/source/blender/render/intern/source/pipeline.c
@@ -722,6 +722,13 @@ void RE_InitState(Render *re, Render *source, RenderData *rd,
                re->r.size = source->r.size;
        }

+       /* disable border if it's a full render anyway */
+       if (re->r.border.xmin == 0.0f && re->r.border.xmax == 1.0f &&
+           re->r.border.ymin == 0.0f && re->r.border.ymax == 1.0f)
+       {
+               re->r.mode &= ~R_BORDER;
+       }
+
        re_init_resolution(re, source, winx, winy, disprect);

        if (re->rectx < 1 || re->recty < 1 || (BKE_imtype_is_movie(rd->im_format.imtype) &&
This revision was automatically updated to reflect the committed changes.