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

Refactor pixel-mapping code for four-scan screens #742

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

j-g00da
Copy link

@j-g00da j-g00da commented Feb 2, 2025

Refactored the pixel-mapping code to use switch instead of nested ifs and improved indentation so it's easier to understand and add new PANEL_SCAN_RATEs in the future.

@j-g00da j-g00da marked this pull request as draft February 2, 2025 12:14
@j-g00da
Copy link
Author

j-g00da commented Feb 2, 2025

This is untested, please test if you have any four-scan screen.
@mrcodetastic @board707

@mrcodetastic
Copy link
Owner

Thanks for this. I will take a look over the coming days.

@board707
Copy link
Contributor

board707 commented Feb 5, 2025

@j-g00da
Thanks for your efforts.

@j-g00da
Copy link
Author

j-g00da commented Feb 5, 2025

Btw. I have 40px-high screens support and settable pixel-base ready, but these depend on this refactor, so I will open further PRs when this is merged.
Then we can create presets for common displays - let me know what do you think about this idea.

@board707
Copy link
Contributor

board707 commented Feb 5, 2025

Then we can create presets for common displays

in fact, there are no "common displays"... there are dozens of varieties of 4xscan matrices, and they all occur with approximately equal frequency.
So, to be honest, I don't see much benefit in creating presets in this part of the code. It would be more correct, as it seems to me, to give the user the ability to easily constructs a pixel mapping for his matrices, which is what I am trying to do in PR #739

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.

3 participants