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

Incorrect color handling in twr_gfx_draw_fill_rectangle_dithering() #320

Closed
mixicz opened this issue Sep 6, 2022 · 2 comments
Closed

Incorrect color handling in twr_gfx_draw_fill_rectangle_dithering() #320

mixicz opened this issue Sep 6, 2022 · 2 comments

Comments

@mixicz
Copy link

mixicz commented Sep 6, 2022

Hi,
when computing pixel color in twr_gfx_draw_fill_rectangle_dithering() function, following code is used:

uint32_t d_color = color & (1 << (dx + 4*dy));

This produces either zero or some positive number in range [1…32768]. This will work fine with monochromatic implementations considering any positive value to be black, but it will break any implementation expecting real color value (for multicolor displays). There is another drawback, which will not allow to choose dithering color.

My suggestion is to fix this function to consistently produce color value of "0" or "1", for example:

uint32_t d_color = (color & (1 << (dx + 4*dy))) >> (dx + 4*dy);

And to introduce another function to allow for multicolor dithering:

void twr_gfx_draw_fill_rectangle_dithering_color(twr_gfx_t *self, int x0, int y0, int x1, int y1, uint16_t pattern, uint32_t color_fg, uint32_t color_bg)
{
    int y;
    for (; x0 <= x1; x0++)
    {
        for (y = y0; y <= y1; y++)
        {
            uint8_t dx = x0 % 4;
            uint8_t dy = y % 4;
            uint32_t d_color = pattern & (1 << (dx + 4*dy)) ? color_fg : color_bg;
            twr_gfx_draw_pixel(self, x0, y, d_color);
        }
    }
}
@SmejkalJakub
Copy link
Contributor

Hello and thank you for your suggestions.

I already added these changes to the GFX SDK module.

I will move these changes to the main branch as soon as possible, I just need to add some comments for the documentation.

The same goes for the second issue that you created, thank you for that as well. I will close both issues when I merge the branch to the main.

Thank you again, for the findings and the fix for both of the issues

@SmejkalJakub
Copy link
Contributor

The new function and the fixes were added to the main branch of the SDK

11ed352

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

No branches or pull requests

2 participants