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

fix: transparent oval outline #763

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

peter-jerry-ye
Copy link

Closes #324

Before:
截屏2024-09-14 10 40 19

After:
截屏2024-09-14 10 40 48

@majaha
Copy link
Contributor

majaha commented Sep 14, 2024

I think the idea here is good, but it doesn't look quite right: your second circle is skinnier than the first one.

There are also several pre-existing weird qualities of the code that I'm going to point out (Not your fault, I just want to call them out as I see them). This code could probably do with a cleanup anyway

@majaha
Copy link
Contributor

majaha commented Sep 14, 2024

From above:

 if (dc1 == 0xf) {
        return;
}

Secret feature to skip the draw? This code seems to be really old, I think it's probably unintentional.

uint8_t strokeColor = (dc1 - 1) & 0x3;
uint8_t fillColor = (dc0 - 1) & 0x3;

This underflows, which was what was causing the dark colour you were seeing. Kinda weird behaviour.

Probably this code needs some real scrutiny and re-writing, but it's complex and people are afraid to touch it so it hasn't gotten any.

@peter-jerry-ye
Copy link
Author

@majaha You are right about the size. I gave it a quick fix by aligning the color to the former one. Though I do think it may need a proper rewrite.

截屏2024-09-14 15 27 32

@aduros aduros added the help wanted Extra attention is needed label Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] oval() does not support transparent outline
3 participants