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

Store round corners in 16x16 patches instead of 8x8 #82

Merged
merged 16 commits into from
Dec 14, 2023
Merged

Conversation

erikcorry
Copy link

This is a moderate performance improvement.
See the 16x16 column below on the example with 4 round buttons on the M5 display, based on tests/icons-visualized.toit.

With diagonal gradient background:

                  Before     Visibility    16x16        Visibility+16x16
Initial draw:     5883ms     2562ms        5336ms       1338ms
Redraw 2 buttons:  840ms     460ms          771ms       370ms

With plain background:

                  Before     Visibility    16x16        Visibility+16x16
Initial draw:     2495ms     1028ms        1043ms       855ms
Redraw 2 buttons:  367ms      271ms        257ms        220ms

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.

src/style.toit Outdated
- byte-opacity: a 64 entry byte array of 8x8 opacity values, or null if the patch is fully transparent.
- bit-opacity: an 8 entry byte array of 8x8 opacity values, or null if the patch is fully transparent.
- byte-opacity: a 256 entry byte array of 16x16 opacity values, or null if the patch is fully transparent.
- bit-opacity: an 32 entry byte array of 16x16 opacity values, or null if the patch is fully transparent.
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
- bit-opacity: an 32 entry byte array of 16x16 opacity values, or null if the patch is fully transparent.
- bit-opacity: a 32 entry byte array of 16x16 opacity values, or null if the patch is fully transparent.

src/style.toit Outdated
- x: the x coordinate of the top left corner of the patch.
- y: the y coordinate of the top left corner of the patch.
- orientation: the orientation to draw the patch, one of $ORIENTATION-0, $ORIENTATION-90, $ORIENTATION-180, $ORIENTATION-270.
*/
draw-corners_ left/int top/int right/int bottom/int corner-radius/int [block]:
for j := 0; j < corner-radius; j += 8:
for i := 0; i < corner-radius; i += 8:
for j := 0; j < corner-radius; j += 16:
Copy link
Member

Choose a reason for hiding this comment

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

You could use constants.
Would make it easier to tweak and give these constants meaning.

src/style.toit Outdated
@@ -423,7 +423,7 @@ class RoundedCornerOpacity_:
radius/int
static cache_ := Map.weak

static OPAQUE-BYTES-8x8_/ByteArray ::= ByteArray 64: 0xff
static OPAQUE-BYTES-16x16_/ByteArray ::= ByteArray 256: 0xff
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure I asked this before, but just didn't see the response: why is this not a literal?

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

In the end I decided not to make this change, because it harmonizes badly with the request to use a constant for the patch size. There's no way to make a literal that is sized based on a different constant.

@erikcorry erikcorry merged commit b3b869c into main Dec 14, 2023
3 checks passed
@erikcorry erikcorry deleted the sixteen branch December 14, 2023 12:36
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