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

"Click to orient" control mode doesn't work correctly on Penrose grid #6

Closed
gordonwoodhull opened this issue Sep 30, 2023 · 6 comments
Milestone

Comments

@gordonwoodhull
Copy link
Owner

          A related problem is that "click to orient" control mode doesn't work correctly on penrose grid.

Originally posted by @gereleth in #2 (comment)

@gordonwoodhull gordonwoodhull changed the title "Click to orient" control mode doesn't work correctly on penrose grid. "Click to orient" control mode doesn't work correctly on Penrose grid Sep 30, 2023
@gordonwoodhull gordonwoodhull added this to the release milestone Sep 30, 2023
@gordonwoodhull
Copy link
Owner Author

gordonwoodhull commented Oct 3, 2023

It's related to #2 but a bit more complicated. The guide dot in other grids relies on the rotation transform, but the Penrose grid doesn't use a rotation transform.

An easy solution would be to apply the tile + rotation transforms only to the dot. However, I think this would look funny because the dot would be squished, especially on the skinny rhombs. So the right way to do this is passing the rotations into getGuideDotPosition() like we are doing for getPipesPath(), and apply the tile transform + rotation to the coordinate using the transformation-matrix object on the polygon.

@gordonwoodhull
Copy link
Owner Author

gordonwoodhull commented Oct 5, 2023

There is something else going on, because sometimes the tiles get colored as if rotation is working, but don't actually move.

image

Confusing!

@gordonwoodhull
Copy link
Owner Author

gordonwoodhull commented Oct 7, 2023

Not seeing the previous behavior, but keeping previous comment in case it returns.

I'm currently seeing:

  1. dot is not necessarily drawn at correct place initially
  2. after clicking on dot, tile will orient to a place where the dot sort of (5) makes sense. but the initial click may also put the tile into negative rotations (4).
  3. after that, clicking clockwise will "wind up" the tile with positive rotations. when the tile has positive rotations, clicking will consistent with moving the dot to that position, although the dot doesn't move.
  4. however, clicking counter-clockwise puts the tile into negative total rotations, and these only display correctly on the even negative numbers. on the odd negative numbers, they are displayed opposite
  5. the dot only sort of makes sense - it's on the correct side, but often displayed toward corner when it should be center, or vice versa

@gereleth
Copy link

gereleth commented Oct 7, 2023

There's definitely something funky going on with directions because I see "connected" tiles having different colors like this:
image

This must mean that the displayed rotation does not correspond to connections state. It's confusing...

I had a hunch that click to orient mode was not aware enough of fix_rotation_offsets but I haven't found anything concrete yet.

Maybe you can remove this mode for the release milestone and work on it at the merge upstream stage)).

@gordonwoodhull
Copy link
Owner Author

gordonwoodhull commented Oct 7, 2023

I think these are all caused by odd negative rotations being being drawn backward. If you spin it clockwise until tileState.data.rotations is positive, my hypothesis is that the colors will repair.

Yeah, I've considered disabling orient/lock for release. We'll see, I'm going to look a little more. This is the last remaining issue for release. 🥳

@gordonwoodhull
Copy link
Owner Author

Okay, I fixed an array rotation bug causing the above backward/reversed tiles, and implemented the tile transform for the dot.

I'll open follow-up issues to implement this correctly for upstream.

I get residual vertigo if I look at the perspective on the dots, but I don't think it's too dangerous to ship.

gordonwoodhull added a commit that referenced this issue Oct 8, 2023
this is part of the orient/lock problem #6
honestly i'm not sure how this works
but i removed a bit of suspicious logic for the negative case
and i would not expect to need a special case for rotating arrays
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

No branches or pull requests

2 participants