Add QuadMesh back as a subclass of PlaneMesh. #65918
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
You should also remove the compat mapping: BTW I noticed two typos worth fixing in "behavior" on this line (en_US spelling + actual typo): |
akien-mga
reviewed
Sep 18, 2022
akien-mga
reviewed
Sep 18, 2022
|
Owh, this is a nice way of dealing with this. Totally see how people are used to using QuadMesh for billboards and screen aligned stuff, while PlaneMesh is usually associated with creating a floor (heightmap). So the difference in default orientation in an otherwise identical implementation is a quality of life improvement. Subclassing solves that nicely. |
This simplifies the creation of billboarded meshes without any code overhead.
|
@akien-mga updated! As a side, typing "behavior" physically hurts me. But it certainly beats "behvaiour" |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #65186
In #64801 we removed the QuadMesh in favour of PlaneMesh. However, this has led to confusion for many users as the button is suddenly gone. Even for users who were aware of the change, the extra steps to get a screen-aligned quad are seen as annoying. Changing the default orientation of PlaneMesh is not a good option as some users prefer it stay the way it is.
Overall, our goal behind #64801 was to remove code overhead and to bring features from PlaneMesh over to QuadMesh (namely subdivision and center). I believe this is achieved by exposing QuadMesh as a subclass of PlaneMesh. QuadMesh will still have all the features of PlaneMesh without us having to maintain two separate classes.
cc @QbieShay @RPicster @BastiaanOlij