Skip to content

Use square for sun in Radwave + other minor tweaks #333

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

Closed
wants to merge 5 commits into from

Conversation

Carifio24
Copy link
Member

This PR does a couple of things:

  • We're currently carrying around two versions of the WWT mode, one which is reactive and one which isn't. This is obviously less than ideal, and we don't actually need the non-reactive version of the mode - WWT knows which mode it's in, and we can find out which one by looking at the type of the background imageset. Thus, this removes the non-reactive mode and queries the background imageset type when we need it outside of the component (only in one place).
  • This aims to resolve Handling of Sun marker in RadWave #331 by using a square to represent the Sun. I was originally planning on doing this via an annotation, but that didn't work due to the way that annotations are filled. I won't get into all the details here, but the short version is that triangulation used for the fill is done by adding the center of the annotation as a "vertex" and doing a fan triangulation from there. This is fine in 2D, but doesn't work so well for our 3D purposes. While that's something I can look at upstream (I don't see any reason why we couldn't use e.g. a Delaunay triangulation instead), it won't be trivial. So for this mini, what I've done is hacked the engine to let us use a solid square of the given color for a spreadsheet layer, rather than drawing a square annotation. This does have the disadvantage that it won't rotate with the camera in 3D. Any thoughts are welcome.

Copy link
Collaborator

@johnarban johnarban left a comment

Choose a reason for hiding this comment

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

This works well for me. The only issues I found was that the patch wasn't applied with yarn serve. The other more important one is the sun square used world scale instead of screen.

@patudom patudom marked this pull request as draft January 8, 2024 21:34
@patudom
Copy link
Contributor

patudom commented Jan 8, 2024

Marking as a draft while we wait for SMEs to decide which approach (this vs. #334) they want for the sun marker.

@patudom
Copy link
Contributor

patudom commented Mar 1, 2024

Closing this because we have released the DS with the gaussian sun instead of changing to a square.

@patudom patudom closed this Mar 1, 2024
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.

Handling of Sun marker in RadWave
3 participants