Skip to content

feat: add basic rotation for 270 degrees#104

Draft
danjenkins wants to merge 2 commits into
andrewjw:mainfrom
everycastlabs:basic-rotation
Draft

feat: add basic rotation for 270 degrees#104
danjenkins wants to merge 2 commits into
andrewjw:mainfrom
everycastlabs:basic-rotation

Conversation

@danjenkins
Copy link
Copy Markdown
Contributor

More angles to be added once we agree on this concept being ok.

This is essentially what I've had to do to be able to create a vertical output using 128x64 display.

I've just moved the logic to the lib.

Its not ideal, but the hub75 driver doesn't support rotation so you have to do it on the buffer level. I think we'd want to only support 0/90/180/270 which is still better than nothing.

Let me know what you think.

More angles to be added once we agree on this
concept being ok
@andrewjw
Copy link
Copy Markdown
Owner

andrewjw commented Oct 5, 2024

This looks great. I'm keen to have rotation supported because although I only have a 64x64, it'd be easier if the power cable was in a different location.

I agree that we only want to support 0, 90, 180 or 270. Anything else just wouldn't work. Can you add an assertion to force it only accept those values?

I've been thinking about how to balance speed and readability. I was thinking of having a set of functions like _rotate_0, _rotate_90 etc, and then in constructor just assigning the correct one to a member variable. You can then call that in the pixel setting functions. You get the downside of a function call even in the 0 degrees case, but avoid comparing to the rotation each time. Hope that makes sense, and happy to accept something else if you think it make sense when adding the rest of the angles.

The last thing is that we should add some tests to verify that pixels get rotated correctly.

@danjenkins
Copy link
Copy Markdown
Contributor Author

Great I'll tackle this soon and get tests/assertions in etc

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