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

[Fix]: ProjectionSystem should clear projectionMatrix #6739

Merged
merged 14 commits into from Jul 4, 2020

Conversation

ShukantPal
Copy link
Member

@ShukantPal ShukantPal commented Jul 2, 2020

https://github.com/pixijs/pixi.js/blob/a4c1e3d58159138f99e2714c476360932ebf0152/packages/core/src/projection/ProjectionSystem.ts#L115

Commenting out pm.identity() was a mistake. The underlying assumption was that we were setting a, d, tx, and ty always; and that b, d would always stay 0.

However, that assumption is false if the user sets a custom projection.transform. If set transform has a non-zero b, d value, then prepending it to the projection-matrix changes the b, d values on it as well.

Demonstration

https://codepen.io/sukantpal/pen/ZEQvKBB?editors=1010

Go to line-43, where I am manually clearing the projection-matrix via renderer.projection.projectionMatrix.identity() and comment it out. The demo message would gradually become larger & larger with no bounds.

ShukantPal and others added 14 commits June 5, 2020 17:15
The projection-system should produce a matrix that converts
world-space coordinates into clip-space coordinates.

The sourceFrame is the rectangle in the scene-graph
(or world) we want to render. It can also be thought of the
"screen" in world-space coordinates.

The clip-space rectangle is always x = -1, 1; y = -1, 1.

The destination frame is just another word for the viewport -
the screen rectangle inside the canvas.
Co-authored-by: Matt Karl <matt@mattkarl.com>
@codecov-commenter
Copy link

Codecov Report

Merging #6739 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               dev     #6739   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          671       671           
=========================================
  Hits           671       671           

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 a4c1e3d...0d68ca3. Read the comment docs.

@bigtimebuddy bigtimebuddy added the ✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t label Jul 3, 2020
@bigtimebuddy bigtimebuddy merged commit c77bc2d into dev Jul 4, 2020
@bigtimebuddy bigtimebuddy deleted the fix/projection-system branch July 4, 2020 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants