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

Possibility to clearing separate color and depth buffers #6358

Merged
merged 13 commits into from
Feb 1, 2020
Merged

Possibility to clearing separate color and depth buffers #6358

merged 13 commits into from
Feb 1, 2020

Conversation

amakaseev
Copy link
Contributor

@amakaseev amakaseev commented Jan 28, 2020

This function required in case when autoclean canvas is disabled for optimisation reason, and I need to draw 3d model on top of 2d. I need to clean only depth buffer before 3d draw. Also it would be useful if you do not use depth buffer, and you do not have to clear it.

@amakaseev amakaseev changed the title Posibility to clearing separate color and depth buffers Possibility to clearing separate color and depth buffers Jan 28, 2020
@codecov-io
Copy link

codecov-io commented Jan 28, 2020

Codecov Report

Merging #6358 into dev will decrease coverage by 2.7%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##              dev   #6358      +/-   ##
=========================================
- Coverage   77.41%   74.7%   -2.71%     
=========================================
  Files         183     105      -78     
  Lines        9613    6053    -3560     
=========================================
- Hits         7442    4522    -2920     
+ Misses       2171    1531     -640
Impacted Files Coverage Δ
...kages/canvas/canvas-renderer/src/CanvasRenderer.js 85.71% <100%> (-0.14%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b568c28...ecd4f4e. Read the comment docs.

@bigtimebuddy
Copy link
Member

@amakaseev did you mean to revert?

@ivanpopelyshev
Copy link
Collaborator

@bigtimebuddy I told @amakaseev in PixiJS slack that API changes are difficult to merge. He's working on better version :)

Copy link
Member

@bigtimebuddy bigtimebuddy left a comment

Choose a reason for hiding this comment

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

Some small doc changes, thanks for the PR. If you could provide and example demonstrating usage, that would be very helpful as well.

packages/core/src/renderTexture/RenderTextureSystem.js Outdated Show resolved Hide resolved
packages/core/src/framebuffer/FramebufferSystem.js Outdated Show resolved Hide resolved
Copy link
Member

@bigtimebuddy bigtimebuddy left a comment

Choose a reason for hiding this comment

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

Thank you for those fixes.

Code-wise and backward compat, I’m good with this. I’ll let others comment on the utility/value of this feature.

@amakaseev
Copy link
Contributor Author

This function required in case when autoclean canvas is disabled for optimisation reason, and I need to draw 3d model on top of 2d. I need to clean only depth buffer before 3d draw. Also it would be useful if you do not use depth buffer, and you do not have to clear it.

@ivanpopelyshev
Copy link
Collaborator

@GoodBoyDigital how is it handled in your fork?

Copy link
Member

@GoodBoyDigital GoodBoyDigital left a comment

Choose a reason for hiding this comment

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

nice 👍

@bigtimebuddy
Copy link
Member

bigtimebuddy commented Jan 31, 2020

@amakaseev could you resolve conflict with dev we just merged in a big PR to core. Hopefully the conflicts here should be minimal.

nvm, I got it!

@bigtimebuddy bigtimebuddy merged commit e0e0cfe into pixijs:dev Feb 1, 2020
@bigtimebuddy
Copy link
Member

Thanks for your contribution @amakaseev 🎉

@amakaseev amakaseev deleted the separate-clearing branch February 3, 2020 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants