Skip to content

Conversation

@lehins
Copy link
Contributor

@lehins lehins commented Jan 5, 2020

From what I gather _paletteSize is an unnecessary field in Palette', since it can be deduced from the size of _paletteData and the type of pixel, which is exactly the way that the newly added paletteSize is implemented. Couple benefits that come from this change:

  • Palette' is now a newtype around Vector, hence slight reduction to memory usage and overall improvement to performance
  • Safety improvement: Exposing private _paletteSize that is coupled to the actual data allows construction of incorrect Palette', which opens a possibility of a segfault when using images converted from such palettes by palettedAsImage

@Twinside
Copy link
Collaborator

At this point, while this is a worthwhile enhancement, I wouldn't break compatibility for such "low" issue, has to keep it in mind in case of bigger breaking change

@lehins
Copy link
Contributor Author

lehins commented Feb 24, 2020

@Twinside I agree, that bundling this together with a bigger breaking change is a good idea.

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