Skip to content

Recompute bounding shapes#2

Merged
dmarcos merged 0 commit into
dmarcos:masterfrom
mrxz:recompute-bounding-shapes
Jul 5, 2023
Merged

Recompute bounding shapes#2
dmarcos merged 0 commit into
dmarcos:masterfrom
mrxz:recompute-bounding-shapes

Conversation

@mrxz
Copy link
Copy Markdown

@mrxz mrxz commented Jun 16, 2023

PR that addresses the underlying problem of aframevr/aframe#5311. Whenever the update method on the TextGeometry is called, the boundingBox and boundingSphere will now be recomputed (if present). This is the same logic that BufferGeometry and LineSegmentsGeometry use.

Note: this PR can't be automatically merged as I rebased it onto the latest master of the original repository (https://github.com/Jam3/three-bmfont-text). This included a couple of nice clean-ups, like the removal of the dependency on https://github.com/Jam3/three-buffer-vertex-data and the object-assign ponyfill. For easier reviewing, please see:

@dmarcos
Copy link
Copy Markdown
Owner

dmarcos commented Jun 16, 2023

This doesn't seem to be mergable (conflicts)

@mrxz
Copy link
Copy Markdown
Author

mrxz commented Jun 17, 2023

This doesn't seem to be mergable (conflicts)

It's rebased on upstream master, didn't want to misrepresent their changes. Easiest would be to pull it in as another branch, GitHub sadly doesn't allow a PR to target a new branch.

@dmarcos
Copy link
Copy Markdown
Owner

dmarcos commented Jun 17, 2023

Are the commits in our fork still relevant? We can perhaps switch back to the original?

@mrxz
Copy link
Copy Markdown
Author

mrxz commented Jun 18, 2023

Are the commits in our fork still relevant? We can perhaps switch back to the original?

Sadly we can't switch back to the original. While some changes in our fork are superseded, the biggest one converting to ES6 classes is still needed (Experience-Monks#41, Experience-Monks#46, Experience-Monks#49)

Not to mention the additional bug fix of recomputing the bounding shapes.

@dmarcos dmarcos merged commit eed4878 into dmarcos:master Jul 5, 2023
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.

2 participants