1179935 - replace Compositor::PrepareViewport with projection info stored on render targets

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: Compositor complex projections, v0

With VR (and possibly other things in the future), we may need to specify a complex viewport and projection.  Instead of extending PrepareViewport and needing to carry this data around, this introduces projection information directly on CompositingRenderTarget which is applied when SetRenderTarget is called.

The default projection is a simple one, like what PrepareViewport normally sets up.  Calling ClearProjection() on a rendertarget returns to this projection.  Calling SetProjection() allows the caller to specify a full projection matrix, znear/zfar, and whether depth buffering should be enabled.

Comment 1

[Markus Stange [:mstange]]

Comment on attachment 8629031 [details] [diff] [review]
Compositor complex projections, v0

Review of attachment 8629031 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/composite/TextureHost.h
@@ +760,5 @@
>    gfx::IntPoint mOrigin;
> +
> +  bool mHasComplexProjection;
> +  gfx::Matrix4x4 mProjectionMatrix;
> +  bool mEnableDepthBuffer;

Might want to move the bools to the end, for better packing.

::: gfx/layers/opengl/CompositingRenderTargetOGL.cpp
@@ +38,4 @@
>    if (mInitParams.mStatus != InitParams::INITIALIZED) {
>      InitializeImpl();
> +    if (mInitParams.mInit == INIT_MODE_CLEAR) {
> +      needsClear = true;

What required this change? Does it fix an existing bug?

::: gfx/layers/opengl/CompositorOGL.cpp
@@ +545,4 @@
>      = static_cast<CompositingRenderTargetOGL*>(aSurface);
>    if (mCurrentRenderTarget != surface) {
>      mCurrentRenderTarget = surface;
> +    if (mCurrentRenderTarget) {

What prompted this change?

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to Markus Stange [:mstange] from comment #1)
> Comment on attachment 8629031 [details] [diff] [review]
> Compositor complex projections, v0
> 
> Review of attachment 8629031 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/composite/TextureHost.h
> @@ +760,5 @@
> >    gfx::IntPoint mOrigin;
> > +
> > +  bool mHasComplexProjection;
> > +  gfx::Matrix4x4 mProjectionMatrix;
> > +  bool mEnableDepthBuffer;
> 
> Might want to move the bools to the end, for better packing.
> 
> ::: gfx/layers/opengl/CompositingRenderTargetOGL.cpp
> @@ +38,4 @@
> >    if (mInitParams.mStatus != InitParams::INITIALIZED) {
> >      InitializeImpl();
> > +    if (mInitParams.mInit == INIT_MODE_CLEAR) {
> > +      needsClear = true;
> 
> What required this change? Does it fix an existing bug?

No -- the code that did this was previously in InitializeImpl, and was removed from there to consolidate it in one place (see the chunk below), to avoid duplicating additional code.

> ::: gfx/layers/opengl/CompositorOGL.cpp
> @@ +545,4 @@
> >      = static_cast<CompositingRenderTargetOGL*>(aSurface);
> >    if (mCurrentRenderTarget != surface) {
> >      mCurrentRenderTarget = surface;
> > +    if (mCurrentRenderTarget) {
> 
> What prompted this change?

The mContextStateTracker.PushOGLSection(gl(), "Frame"); was removed just below (from BeginFrame), again to consolidate code so that SetRenderTarget() could do extra work without needing to duplicate it twice in BeginFrame. A compositor frame starts with mCurrentRenderTarget == nullptr, so this would cause a mismatched Pop() otherwise.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa2405094adb

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fa2405094adb