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

Add way to improve performance by using more memory #96

Merged
merged 5 commits into from
Mar 15, 2024
Merged

Conversation

erikcorry
Copy link

No description provided.

@erikcorry erikcorry requested a review from floitsch March 15, 2024 09:35
Copy link
Member

@floitsch floitsch left a comment

Choose a reason for hiding this comment

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

LGTM.

multiple color components, transparency, and clipping, so the
effective memory use may be higher than this number.
*/
max-patch-size/int := 2000
Copy link
Member

Choose a reason for hiding this comment

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

Maybe make it := ? and always set it in the constructor?
Otherwise you might get misled by just looking here (and not realizing that some displays, like the two-color one) set it to a different value.

You could also have TWO-COLOR-DEFAULT-PATCH-SIZE... constants.

@@ -77,6 +77,17 @@ abstract class PixelDisplay implements Window:
inner-width: return driver_.width
inner-height: return driver_.width

/**
By default, the display is rendered in patches that have
a max size of 2-8k, depending on the bits per pixel. On devices
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
a max size of 2-8k, depending on the bits per pixel. On devices
a max size of 2-8k pixels, depending on the bits per pixel. On devices

Copy link
Member

Choose a reason for hiding this comment

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

From a user's point of view setting the memory used might be easier. Like "you are allowed to use 20K of RAM for each patch". Not really sure about that.

Copy link
Author

Choose a reason for hiding this comment

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

That's what it is already though.

Copy link
Member

Choose a reason for hiding this comment

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

Why does the TwoColorPixelDisplay then set it to a higher value? I would have expected the 2-color display to use the least amount of memory.

@erikcorry erikcorry merged commit 96d8f7d into main Mar 15, 2024
3 checks passed
@erikcorry erikcorry deleted the higher-memory branch March 15, 2024 12:25
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