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

RESOLVED FIXED in Firefox 42

Status

()

Core
Graphics: Layers
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: vlad, Assigned: vlad)

Tracking

Trunk
mozilla42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

(Whiteboard: [gfx-noted][vrm2])

Attachments

(1 attachment)

Created attachment 8629031 [details] [diff] [review]
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.
Attachment #8629031 - Flags: review?(mstange)
QA Contact: vladimir
Assignee: nobody → vladimir
Whiteboard: [gfx-noted]
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?
Attachment #8629031 - Flags: review?(mstange) → review+
(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.

Updated

2 years ago
Whiteboard: [gfx-noted] → [gfx-noted][vrm2]

Comment 3

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa2405094adb
https://hg.mozilla.org/mozilla-central/rev/fa2405094adb
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.