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.

Sign up for GitHub

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

Jump to bottom

Remove the IR (internal representation) part of the ColorSpace parsing #12083

Merged
merged 2 commits into from
Jul 10, 2020
Merged

Remove the IR (internal representation) part of the ColorSpace parsing #12083

merged 2 commits into from
Jul 10, 2020

Commits on Jul 10, 2020

  1. Remove the IR (internal representation) part of the ColorSpace parsing 

    Originally ColorSpaces were only *partially* parsed on the worker-thread, to obtain an IR-format which was sent to the main-thread. This had the somewhat unfortunate side-effect of causing the majority of the (potentially heavy) ColorSpace parsing to happen on the main-thread.
Hence PR 4824 which, among other things, changed ColorSpaces to be *fully* parsed on the worker-thread with only RGB-data being sent to the main-thread.

While it thus originally was necessary to have `ColorSpace.{parseToIR, fromIR}` methods, to handle the worker/main-thread split, that's no longer the case and we can thus reduce all of the ColorSpace parsing to one method instead.

Currently, when parsing a ColorSpace, we call `ColorSpace.parseToIR` which parses the ColorSpace-data from the document and then creates the IR-format. We then, immediately, call `ColorSpace.fromIR` which parses the IR-format and then finally creates the actual ColorSpace.[1]
All-in-all, this leads to a fair amount of unnecessary indirection which also (in my opinion) makes the code less clear.

Obviously these changes are not really expected to have a significant effect on performance, especially with the recently added caching of ColorSpaces, however there'll now be strictly fewer function calls, less memory allocated, and overall less parsing required during ColorSpace-handling.

---
[1] For ICCBased ColorSpaces, given the validation necessary, this currently even leads to parsing an /Alternate ColorSpace *twice*.
    @Snuffleupagus
    Snuffleupagus committed Jul 10, 2020
    Configuration menu
    Copy the full SHA
    ea6a0e4 View commit details
    Browse the repository at this point in the history

  2. Remove the special handling, used when creating Indexed ColorSpaces, … 

    …for the case where the `lookup`-data is a `Stream`

This special-case was added in PR 1992, however it became unnecessary with the changes in PR 4824 since all of the ColorSpace parsing is now done on the worker-thread (with only RGB-data being sent to the main-thread).
    @Snuffleupagus
    Snuffleupagus committed Jul 10, 2020
    Configuration menu
    Copy the full SHA
    d18cf47 View commit details
    Browse the repository at this point in the history