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

Use rel for cartographic polygon #648

Merged
merged 2 commits into from
Jan 30, 2024
Merged

Conversation

corybarr
Copy link
Contributor

@corybarr corybarr commented Jan 30, 2024

Resolves #639
Fixes #632

Based on the plan to add the CesiumCartographicPolygonPrim back into the codebase once the issue with BasisCurves inheritance is fixed, these are the minimal set of code changes to achieve removing the prim. To the end user, the "cartographic polygon" concept and terminology still exists. The term is still used in the UI. The only difference to the end user once the prim is added back should be the type of the prim in the Stage Window.

This means that OmniCartographicPolygon still exists, even though there is no corresponding USD prim.

Test file: noCartographicPolygon.clippingTest.TwoOverlays.usda.zip

@r-veenstra
Copy link
Contributor

I've been testing and appears to be working as expected. All I could probably suggest is a warning to the log if a user links a basis curve without a globe anchor (something I've already done by accident).

@corybarr
Copy link
Contributor Author

I've been testing and appears to be working as expected. All I could probably suggest is a warning to the log if a user links a basis curve without a globe anchor (something I've already done by accident).

I can see the use of that. Let's create an issue during standup if we want to. I think our rel workaround is good as workarounds go, but ultimately we should inherit from a BasisCurves so we can define the GlobeAnchor requirement in the schema. So any warnings would be temporary with an issue that keeps us from forgetting to remove them in the future.

Copy link
Contributor

@lilleyse lilleyse left a comment

Choose a reason for hiding this comment

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

Looks great! Nothing from me.

@corybarr corybarr merged commit cde0bcf into main Jan 30, 2024
3 checks passed
@corybarr corybarr deleted the use-rel-for-cartographic-polygon branch January 30, 2024 19:50
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.

Use rel for CesiumCartographicPolygon Cartographic Polygon editing crash - fabric only
3 participants