Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render via an intermediate framebuffer #16996

Merged
merged 5 commits into from Sep 4, 2019

Conversation

pchote
Copy link
Member

@pchote pchote commented Aug 26, 2019

This PR kicks off the next set of renderer changes, which I'm hoping to complete during the Next + 1 cycle. This sets up a frame buffer to render everything into, which is then displayed as a quad over the game window. The screenshot code has been rewritten (removing the legacy GL2-only code) to fetch the texture data from the frame buffer.

This sets the foundation for two sets of future PRs:

OpenRA.Game/Renderer.cs Outdated Show resolved Hide resolved
OpenRA.Game/Renderer.cs Outdated Show resolved Hide resolved
OpenRA.Platforms.Default/ThreadedGraphicsContext.cs Outdated Show resolved Hide resolved
@pchote
Copy link
Member Author

pchote commented Aug 27, 2019

This is going to need a fairly significant rework to behave properly wrt HiDPI - hold off reviewing until this is fixed.

@pchote
Copy link
Member Author

pchote commented Aug 27, 2019

Updated.

-depthScale * zoom / screen.Height);
shader.SetVec("r2", -1, 1, 1 - depthOffset);
shader.SetVec("r2", -1, -1, 1 - depthOffset);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before anyone asks: this change and the EnableScissor rename are removing the hardcoded y coordinate flip that used to be baked in to the rendering to work around OpenGL's origin being the bottom left of the window. We now preserve coordinates through the frame buffer(s) and do the flip at the end by rendering the screen quad upside down.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

OpenRA.Game/Renderer.cs Outdated Show resolved Hide resolved
@pchote pchote force-pushed the renderer-framebuffer branch 2 times, most recently from 8327a3a to 95b79ee Compare August 28, 2019 08:21
@pchote
Copy link
Member Author

pchote commented Aug 28, 2019

Fixed + fixed another issue that caused the framebuffer to be recreated each frame.

Copy link
Contributor

@teinarss teinarss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also getting these artifacts on my Dell xps 15, runnig with DisableWindowsDPIScaling: False
Windows 10 64bits.
image

OpenRA.Game/Renderer.cs Outdated Show resolved Hide resolved
OpenRA.Game/Renderer.cs Outdated Show resolved Hide resolved
@pchote
Copy link
Member Author

pchote commented Aug 28, 2019

Fixed.

teinarss
teinarss previously approved these changes Aug 29, 2019
OpenRA.Game/Renderer.cs Outdated Show resolved Hide resolved
OpenRA.Game/Renderer.cs Outdated Show resolved Hide resolved
OpenRA.Game/Renderer.cs Outdated Show resolved Hide resolved
@pchote
Copy link
Member Author

pchote commented Aug 31, 2019

Fixed.

teinarss
teinarss previously approved these changes Aug 31, 2019
Copy link
Contributor

@teinarss teinarss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, cant comment on the opengl details. Tested on the same systems as before. no regression what i could detect.

OpenRA.Game/Renderer.cs Show resolved Hide resolved
OpenRA.Game/Game.cs Show resolved Hide resolved
-depthScale * zoom / screen.Height);
shader.SetVec("r2", -1, 1, 1 - depthOffset);
shader.SetVec("r2", -1, -1, 1 - depthOffset);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

// Render the compositor buffer to the screen
// HACK / PERF: Fudge the coordinates to cover the actual window while keeping the buffer viewport parameters
// This saves us two redundant (and expensive) SetViewportParams each frame
RgbaSpriteRenderer.DrawSprite(screenSprite, new float3(0, lastBufferSize.Height, 0), new float3(lastBufferSize.Width, -lastBufferSize.Height, 0));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use BlitFramebuffer here to avoid the viewport nonsense

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glBlitNamedFramebuffer doesn't seem to be available from EXT_framebuffer_object, so this will have to wait until/after #17010

@teinarss
Copy link
Contributor

teinarss commented Sep 1, 2019

Still looks good, tested after the updates.

@pchote
Copy link
Member Author

pchote commented Sep 3, 2019

I have several more PRs now being squeezed between this and my next overseas travel. Can we please prioritize getting this merged ASAP before my momentum screeches to a complete halt?

@teinarss teinarss dismissed RoosterDragon’s stale review September 4, 2019 18:19

Not sure why this is still here.

@teinarss teinarss merged commit 1d106e7 into OpenRA:bleed Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants