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

Allow 2D panel to be larger than 256px in both dimensions #4044

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lexszero
Copy link

@lexszero lexszero commented Jul 6, 2024

Increase MAX_LEDS_PER_BUS and change width and height type in WS2812FX class to uint16_t from uint8_t, which overflowed when attempted to configure a panel larger than 255 pixels.

@blazoncek
Copy link
Collaborator

The use of uint8_t is intentional.

@lexszero
Copy link
Author

lexszero commented Jul 7, 2024

The use of uint8_t is intentional.

Interesting, what's the purpose and what this change will break?
And how I should go about setting up 300x10 matrix? What I saw is width overflowed uint8_t resulting in my matrix being 44x10 - this change makes it work correctly.

@blazoncek
Copy link
Collaborator

If you want this to be included, modify to be ESP32 only.
Verify other parts of the code not to break anything (UI, JSON API, etc). You'll need to check and discover yourself.

@softhack007
Copy link
Collaborator

softhack007 commented Jul 7, 2024

@lexszero please clarify how you tested your proposed modification.

Did you check that all 2D effects work as before? What about effects like drip or popcorn, which have a special "column mode" when bar is selected as mapping?

I would expect that some 2D effects - like octopus or crazy bee - will not work when rows or cols is greater than 255.

@softhack007
Copy link
Collaborator

@lexszero important: please rebase your PR to our 0_15 branch.

@softhack007 softhack007 added discussion rebase needed This PR needs to be re-based to the current development branch labels Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion rebase needed This PR needs to be re-based to the current development branch
3 participants