Skip to content
This repository has been archived by the owner on Jan 25, 2023. It is now read-only.

Allow Camera::SetFlipVertical to be used externally for OpenGL #2643

Conversation

ChrisDenham
Copy link
Contributor

@ChrisDenham ChrisDenham commented Jun 6, 2020

Allow Camera::SetFlipVertical to be used externally for OpenGL by toggling it either side of render to texture, i.e. instead of "set and clear" either side of render to texture.

The purpose of this pull request is to allow Camera::SetFlipVertical to be used for both DirectX and OpenGL to switch into a right handed coordinate system (see #2642).

This pull request should not change the behaviour of any application that does not call Camera::SetFlipVertical directly. It also should correct the scenario where if the render to texture should not be flipped if the camera is already flipped (hence the toggling of state in the changes)

Closes #2642

…ng it either side of render to texture, i.e. instead of "set and clear" either side of render to texture.

(cherry picked from commit 434739d62c99e9328b66c338b7dc57c963b586c6)
@eugeneko
Copy link
Contributor

eugeneko commented Jun 6, 2020

If no objections arise, I'll merge it in three days

@Lumak
Copy link
Contributor

Lumak commented Jun 10, 2020

for every render target. flipping is toggled

camera_->SetFlipVertical(!camera_->GetFlipVertical());

so every other render target is flipped?

just looked at the code, and this exists in Render() function, so it's flipped every other frame?

@ChrisDenham
Copy link
Contributor Author

No, I don't think so. My change toggles the flip camera state either side of the render for OGL. I believe this is equivalent to the original code which sets then clears the flipped state. The purpose of my PR is simply to retain the flipped state of the camera set by the user, which was previously being lost every frame for OGL.

@1vanK
Copy link
Contributor

1vanK commented Jun 10, 2020

Yes, the code does not look intuitive. May be add some more comment, that the value restored below.

@ChrisDenham
Copy link
Contributor Author

Yes, the code does not look intuitive. May be add some more comment, that the value restored below.

Yes, I agree, it's not immediately obvious why it's a toggle or why the restoration is a toggle.
How's this amended comment?

#ifdef URHO3D_OPENGL
    if (renderTarget_)
    {
        // On OpenGL, flip the projection if rendering to a texture so that the texture can be addressed in the same way
        // as a render texture produced on Direct3D9
        // Note that the state of the FlipVertical mode is toggled here rather than enabled
        // The reason for this is that we want the mode to be the opposite of what the user has currently set for the
        // camera when rendering to texture for OpenGL
        // This mode is returned to the original state by toggling it again below, after the render
        if (camera_)
            camera_->SetFlipVertical(!camera_->GetFlipVertical());
    }
#endif

(cherry picked from commit 4f1ca6cf8595b230e7ab2473fc37d1c0325a481a)
@Lumak
Copy link
Contributor

Lumak commented Jun 10, 2020

Yes, the comment is very informative. I get it now.

@ChrisDenham
Copy link
Contributor Author

Can this PR be merged now? Or are there any outstanding concerns about it?

@eugeneko
Copy link
Contributor

Yeah, I tend to say "I'll merge it in 3 days" and forget about it for weeks.

@eugeneko eugeneko merged commit 6fb55fa into urho3d:master Jun 14, 2020
urho3d-travis-ci added a commit that referenced this pull request Jun 14, 2020
* Allow Camera::FlipVertical to be used externally for OpenGL by toggling it either side of render to texture, i.e. instead of "set and clear" either side of render to texture.

(cherry picked from commit 434739d62c99e9328b66c338b7dc57c963b586c6)

* #1 Add explanation comment to previous commit.

(cherry picked from commit 4f1ca6cf8595b230e7ab2473fc37d1c0325a481a)

Co-authored-by: Chris Denham <chris.denham@logicom.com>
urho3d-travis-ci added a commit that referenced this pull request Jun 14, 2020
* Allow Camera::FlipVertical to be used externally for OpenGL by toggling it either side of render to texture, i.e. instead of "set and clear" either side of render to texture.

(cherry picked from commit 434739d62c99e9328b66c338b7dc57c963b586c6)

* #1 Add explanation comment to previous commit.

(cherry picked from commit 4f1ca6cf8595b230e7ab2473fc37d1c0325a481a)

Co-authored-by: Chris Denham <chris.denham@logicom.com>
urho3d-travis-ci added a commit that referenced this pull request Jun 14, 2020
* Allow Camera::FlipVertical to be used externally for OpenGL by toggling it either side of render to texture, i.e. instead of "set and clear" either side of render to texture.

(cherry picked from commit 434739d62c99e9328b66c338b7dc57c963b586c6)

* #1 Add explanation comment to previous commit.

(cherry picked from commit 4f1ca6cf8595b230e7ab2473fc37d1c0325a481a)

Co-authored-by: Chris Denham <chris.denham@logicom.com>
urho3d-travis-ci added a commit that referenced this pull request Jun 14, 2020
* Allow Camera::FlipVertical to be used externally for OpenGL by toggling it either side of render to texture, i.e. instead of "set and clear" either side of render to texture.

(cherry picked from commit 434739d62c99e9328b66c338b7dc57c963b586c6)

* #1 Add explanation comment to previous commit.

(cherry picked from commit 4f1ca6cf8595b230e7ab2473fc37d1c0325a481a)

Co-authored-by: Chris Denham <chris.denham@logicom.com>
urho3d-travis-ci added a commit that referenced this pull request Jun 14, 2020
* Allow Camera::FlipVertical to be used externally for OpenGL by toggling it either side of render to texture, i.e. instead of "set and clear" either side of render to texture.

(cherry picked from commit 434739d62c99e9328b66c338b7dc57c963b586c6)

* #1 Add explanation comment to previous commit.

(cherry picked from commit 4f1ca6cf8595b230e7ab2473fc37d1c0325a481a)

Co-authored-by: Chris Denham <chris.denham@logicom.com>
urho3d-travis-ci added a commit that referenced this pull request Jun 14, 2020
* Allow Camera::FlipVertical to be used externally for OpenGL by toggling it either side of render to texture, i.e. instead of "set and clear" either side of render to texture.

(cherry picked from commit 434739d62c99e9328b66c338b7dc57c963b586c6)

* #1 Add explanation comment to previous commit.

(cherry picked from commit 4f1ca6cf8595b230e7ab2473fc37d1c0325a481a)

Co-authored-by: Chris Denham <chris.denham@logicom.com>
urho3d-travis-ci added a commit that referenced this pull request Jun 14, 2020
* Allow Camera::FlipVertical to be used externally for OpenGL by toggling it either side of render to texture, i.e. instead of "set and clear" either side of render to texture.

(cherry picked from commit 434739d62c99e9328b66c338b7dc57c963b586c6)

* #1 Add explanation comment to previous commit.

(cherry picked from commit 4f1ca6cf8595b230e7ab2473fc37d1c0325a481a)

Co-authored-by: Chris Denham <chris.denham@logicom.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to change from left to right handed coordinate system?
4 participants