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

Throw error when canvas background is used without sample buffer #31751

Merged
merged 1 commit into from Sep 23, 2019

Conversation

@clayjohn
Copy link
Contributor

clayjohn commented Aug 29, 2019

Fixes: #16226

edit: I have updated this PR with a completely different approach. After spending some more time with it, I realized that the render_empty_scene call is needed to actually render the canvas background. So the real problem is that in order to use a canvas background, by nature you have to sample the main buffer. So using "2D - No Sampling" shouldn't be allowed. My new solution is to throw an error and to exit the render_scene function.

@akien-mga akien-mga added this to the 3.2 milestone Aug 29, 2019
@akien-mga akien-mga requested a review from reduz Aug 29, 2019
@akien-mga

This comment has been minimized.

Copy link
Member

akien-mga commented Sep 1, 2019

Note that render_empty_scene becomes unused after this PR, so it should also be removed.

From @reduz's comment it's not clear whether it's the right solution though:

14:36 <Akien> reduz: OK with #31751 (bugfix)?
14:36 <IssueBot> #31751: Remove `render_empty_scene` when Viewports cant draw 3d | https://git.io/fjxCk
14:36 <Akien> It's not clear why this render_empty_scene was used in the first place
14:37 <reduz> it exists for when you use an environment with a canvas BG
14:39 <Akien> Maybe the bool was meant to be `can_draw_3d` instead of its negation?
14:39 <Akien> See https://github.com/godotengine/godot/issues/16226#issuecomment-520125123
18:50 <clayjohn> reduz: Akien: I removed render_empty_scene because it was causing a crash in GLES3 and it caused the screen to go black in GLES2. Is it just being used to clear the background and optionally draw a sky?
@clayjohn clayjohn force-pushed the clayjohn:GLES3-Viewport-crash-canvas branch from 7863f62 to e65d218 Sep 16, 2019
@clayjohn clayjohn changed the title Remove `render_empty_scene` when Viewports cant draw 3d Throw error when canvas background is used without sample buffer Sep 16, 2019
@akien-mga akien-mga merged commit f1146c2 into godotengine:master Sep 23, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@akien-mga

This comment has been minimized.

Copy link
Member

akien-mga commented Sep 23, 2019

Thanks!

@akien-mga

This comment has been minimized.

Copy link
Member

akien-mga commented Nov 8, 2019

Cherry-picked for 3.1.2.

@clayjohn clayjohn deleted the clayjohn:GLES3-Viewport-crash-canvas branch Nov 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.