Compositor/Viewport/Camera issue

Discussion area about developing or extending OGRE, adding plugins for it or building applications on it. No newbie questions please, use the Help forum for that.
Post Reply
User avatar
lf3thn4d
Orc
Posts: 478
Joined: Mon Apr 10, 2006 9:12 pm

Compositor/Viewport/Camera issue

Post by lf3thn4d » Tue Oct 27, 2009 3:42 pm

Currently compositor is strictly tied to a viewport and camera. All is well. However, problem arises when one swaps camera on the given viewport. The compositor chain's internal RT still points to the old camera hence will not render the actual active camera. The current workaround is to disable all compositors and re-enabling them back. I don't quite like this idea being that it means destroying RTs and recreating them. This is quite unnecessary as all that is needed is to update the target camera on each RTs of compositors.

Suggestion:
1. Have function call to update camera to all RT that users can call when they change camera of viewport.
2. Make it automatic such that when viewport's camera changes, compositor will automatically update itself.
0 x

User avatar
sinbad
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 19261
Joined: Sun Oct 06, 2002 11:19 pm
Location: Guernsey, Channel Islands
Contact:

Re: Compositor/Viewport/Camera issue

Post by sinbad » Wed Oct 28, 2009 4:40 pm

lf3thn4d wrote:The current workaround is to disable all compositors and re-enabling them back. I don't quite like this idea being that it means destroying RTs and recreating them. This is quite unnecessary as all that is needed is to update the target camera on each RTs of compositors.
Actually it doesn't if you mark those RTs as 'pooled' - such textures are not destroyed but returned to a pool and re-used, this is also very useful when you switch compositors on / off for temporary effects etc. When I added this it was called 'shared' but that has been changed to 'pooled' with the newest compositor changes which is a better term.
Suggestion:
1. Have function call to update camera to all RT that users can call when they change camera of viewport.
2. Make it automatic such that when viewport's camera changes, compositor will automatically update itself.
I like 2 better, since if you're going to change something you might as well make it automatic. It might be a case for an explicit Viewport::Listener which doesn't currently exist.
0 x

User avatar
lf3thn4d
Orc
Posts: 478
Joined: Mon Apr 10, 2006 9:12 pm

Re: Compositor/Viewport/Camera issue

Post by lf3thn4d » Thu Oct 29, 2009 2:39 am

Ah.. ok. :) I need to read up on new compositor.

Yes, 2 is a better choice.

However, we still need to deal with the case of NULL camera. In fact, the compositor now crashes if you try to enable compositor instance of a viewport with no camera attached. Obviously there would need to be a better handling of this case if solution 2 is to be done.

We can either do (when camera is NULL):
A: Ignore all camera steps.
B: Ignore camera steps when enabling RTs then skip compositor all together when rendering.
C: Auto disable whole compositor chain. (A bit flaky and feels like an unwanted behavior)

I personally kind of prefer A. Reason being that it means compositor would be free of the constraint of camera. This would mean it could even be used for custom 2D renderers that uses the Ogre's viewport update system (with some smartypants hijacking to setup proper viewport callback to RTs). I liked this idea because I actually wrote a 2D engine that fits this criteria. :P
0 x

User avatar
sinbad
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 19261
Joined: Sun Oct 06, 2002 11:19 pm
Location: Guernsey, Channel Islands
Contact:

Re: Compositor/Viewport/Camera issue

Post by sinbad » Fri Oct 30, 2009 3:24 pm

Funnily enough I had a situation today where I could have used a compositor with no camera attached. So A gets my vote.
0 x

User avatar
lf3thn4d
Orc
Posts: 478
Joined: Mon Apr 10, 2006 9:12 pm

Re: Compositor/Viewport/Camera issue

Post by lf3thn4d » Fri Oct 30, 2009 5:59 pm

Haha. Alrights. A it is then. :) patch coming up. :D

btw, can we have updated manuals in trunk? Pre-generated manuals on compositor doesn't talk about pooled and shared RTs.
0 x

User avatar
lf3thn4d
Orc
Posts: 478
Joined: Mon Apr 10, 2006 9:12 pm

Re: Compositor/Viewport/Camera issue

Post by lf3thn4d » Sat Oct 31, 2009 10:00 am

Question:
I'm attempting to add new Listener interface for Viewport. Should it support multiple listener or only one?
The plan is to have setCamera and setDimensions trigger the listener. Then from CompositorChain that listens to viewport updates CompositorInstance base on change.
If setCamera() was called, all RT will update their viewport according to camera change. If setDimensions() was called, CompositorInstance::notifyResized() will be called.
0 x

User avatar
Noman
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 714
Joined: Mon Jan 31, 2005 7:21 pm
Location: Israel
Contact:

Re: Compositor/Viewport/Camera issue

Post by Noman » Sat Oct 31, 2009 1:33 pm

lf3thn4d wrote:Haha. Alrights. A it is then. :) patch coming up. :D

btw, can we have updated manuals in trunk? Pre-generated manuals on compositor doesn't talk about pooled and shared RTs.
Hrm. I think sinbad only updates the actual 'compiled' manual before releases (or release candidates etc). I never got around to building a 'complete' manual compilation environment. I could always compile and see that the individual pages get generated correctly, but never in the exact organizational format that the ogre manual is.
0 x

User avatar
lf3thn4d
Orc
Posts: 478
Joined: Mon Apr 10, 2006 9:12 pm

Re: Compositor/Viewport/Camera issue

Post by lf3thn4d » Sat Oct 31, 2009 5:46 pm

Oh.. that's rather odd since Trunk is always the bleeding edge stuffs. I guess no choice but to find a way to build it myself somehow. I just needed to refer to the new pooling and shared RT type resources so I can get my bearing right on which to use on what and when.

[edit]
After working on this more, I realised there's more than meets the eye here. Here's what I'm going to be doing:
1. Add viewport listener for camera change, dimension change and viewport destruction.
2. Make compositor ignore camera settings if camera is NULL.
3. Update compositor instance of camera and dimension changes through listener.
4. Remove compositor chain when viewport destructs through listener.
5. Remove _notifyViewport() from compositor chain since no longer needed because of 4.
[/edit]

[edit2]
On second thought, the RenderTargetListener already triggers viewport destruction. Why is it not using that to remove compositor chain?
[/edit2]

[edit3]
I'm planning to make CompositorChain use viewport's destruction event instead of RenderTargetListener's event since we want specific viewport. This is also solve for the case where viewport is destroyed before CompositorChain gets to initialize which will not trigger the viewport destruction callback.

Aside from this, I'm suggesting to add CompositorManager pointer to CompositorChain so that it can remove itself when viewport is destroyed. Or else, have CompositorManager do a prune pass during getCompositorChain(). Personally, I prefer first method. Going to do that unless there's objection.
[/edit3]

[edit4]
Hmm.. no, CompositorManager is a singleton. No reason of not being able to access it from CompositorChain. So viewport destruction event should be able to destroy CompositorChain.
[/edit4]
0 x

User avatar
lf3thn4d
Orc
Posts: 478
Joined: Mon Apr 10, 2006 9:12 pm

Re: Compositor/Viewport/Camera issue

Post by lf3thn4d » Mon Nov 02, 2009 9:41 am

Patch: https://sourceforge.net/tracker/?func=d ... tid=302997
Changes:
- Compositor can now execute without camera attached to viewport.
- Camera change in viewport will automatically be propagated to compositor RTs.
- Viewport destruction will automatically remove CompositorChain from manager.
- Removed _notifyViewport() from CompositorChain since no longer needed.

Note: Even though this patch made compositor work with camera-less viewports, due to the way CompositionPass is executed(using scenemanager's render queue), it's rather useless to be used for 2D stuffs as described by my post above. Probably need to refit composition pass execution technique.
0 x

User avatar
lf3thn4d
Orc
Posts: 478
Joined: Mon Apr 10, 2006 9:12 pm

Re: Compositor/Viewport/Camera issue

Post by lf3thn4d » Fri Jan 29, 2010 10:27 pm

*bump*

I hope this is not forgotten. :-P Sorry for the bump.
0 x

User avatar
sinbad
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 19261
Joined: Sun Oct 06, 2002 11:19 pm
Location: Guernsey, Channel Islands
Contact:

Re: Compositor/Viewport/Camera issue

Post by sinbad » Sat Jan 30, 2010 7:10 pm

Not forgotten, it's just more problematic since you chose to remove a bunch of safety checks in the patch which means it's more cumbersome for me to test fully (compared to just new functionality). I haven't had time yet.

Basically, the easier a patch is for me to test, both the new functionality and that it doesn't break anything else, the faster it gets processed. That's why it's generally best to stick to smaller scope, focussed patches and not to roll up 'other' changes at the same time.
0 x

User avatar
Noman
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 714
Joined: Mon Jan 31, 2005 7:21 pm
Location: Israel
Contact:

Re: Compositor/Viewport/Camera issue

Post by Noman » Mon Feb 01, 2010 9:02 am

Hi

I have some available time in the next few days so I can take a close look at it. Can you update the patch so it will patch against the current SVN? I currently get a few conflicts when I try to apply it...
Also, is this patch intended for 1.7 or trunk? I don't think any APIs are changed by this, but its still more than a bugfix...
0 x

User avatar
lf3thn4d
Orc
Posts: 478
Joined: Mon Apr 10, 2006 9:12 pm

Re: Compositor/Viewport/Camera issue

Post by lf3thn4d » Mon Feb 01, 2010 4:43 pm

@Sinbad: Really sorry for the problematic change.

@Noman: Thanks :) I'll work on a new patch asap. It's meant for both 1.7 and trunk. It basically resolves the issue of the reliance of camera in the construction of compositors. In terms of behavior changes, codes that are already working shouldn't have any issues. This patch mostly just made sure compositors can be added without a camera attached to the given viewport. And also smartly propagate the camera change of the main viewport to the RT's viewports.
0 x

User avatar
lf3thn4d
Orc
Posts: 478
Joined: Mon Apr 10, 2006 9:12 pm

Re: Compositor/Viewport/Camera issue

Post by lf3thn4d » Thu Feb 04, 2010 8:50 pm

Ok. new patch is submitted.
0 x

User avatar
Noman
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 714
Joined: Mon Jan 31, 2005 7:21 pm
Location: Israel
Contact:

Re: Compositor/Viewport/Camera issue

Post by Noman » Sun Feb 07, 2010 1:06 pm

I was going to check this patch out and commit it but sinbad beat me to it :)
0 x

User avatar
lf3thn4d
Orc
Posts: 478
Joined: Mon Apr 10, 2006 9:12 pm

Re: Compositor/Viewport/Camera issue

Post by lf3thn4d » Sun Feb 07, 2010 5:14 pm

Haha. yeah. awesomeness either way. :D Thanks for accepting this patch. Now this plus that shared depth buffer patch would make compositor very useful for proper deferred shading. :D
0 x

Post Reply