Skip to content

lv-tool: Fix check for a suitable palette - #215

Closed
hartwork wants to merge 1 commit into
masterfrom
lv-tool-fix-palette-application-guard
Closed

hartwork wants to merge 1 commit into
masterfrom
lv-tool-fix-palette-application-guard

Conversation

@hartwork

@hartwork hartwork commented Jan 24, 2023 •

Copy link
Copy Markdown
Member

We can only copy 256 entries if we have them...

(based on 0.4.x commit d1edda9 from pull request #213)

We can only copy 256 entries if we have them...

(based on 0.4.x commit d1edda9)

@kaixiong kaixiong left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@hartwork, I don't think this patch is correct. It's clearly intended to copy palettes with smaller than 256 entries and fill the remaining with black.

What's slightly questionable in this implementation is the use of visual_mem_set() to zero out the entries but it should be safe since SDL_Color is just a regular C struct defined with uint8_t (unless I'm missing something about std::array's guarantees regarding memory layout).

@hartwork

Copy link
Copy Markdown
Member Author

@hartwork, I don't think this patch is correct. It's clearly intended to copy palettes with smaller than 256 entries and fill the remaining with black.

@kaixiong you're right, sorry for not having checked in depth. Thanks for catching this! 👍 Closing…

@hartwork hartwork closed this Jan 24, 2023
@hartwork
hartwork deleted the lv-tool-fix-palette-application-guard branch January 24, 2023 20:08
@hartwork hartwork removed this from the 0.5.0_alpha1 milestone Jan 24, 2023
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