lovell / icc

JavaScript module to parse International Color Consortium (ICC) profiles
Apache License 2.0
57 stars 6 forks source link

Support more types #3

Closed xi closed 1 year ago

xi commented 5 years ago

Fixes #2

xi commented 5 years ago

Hi, thanks for the quick response. I was thinking about the tests: For some reasons they are hanging on my laptop. I think this could be because using a deepEqual on the (now considerably larger) output takes too long.

Also, I am not sure about the API itself:

This tagging scheme allows developers to read in the element tag table and then randomly access and load into memory only the information necessary to their particular software application. Since some instances of profiles can be quite large, this provides significant savings in performance and memory. ­— ICC.1:2001-04 section 0.5

So maybe it would be better to define a on-demand getTag() functon instead of eagerly parsing all tags.

I am still thinking about what is the best approach here. So you can consider this a work in progress. I wanted to push as ealy as possible to get your opinion on it.

xi commented 5 years ago

Another thing I am thinking about: Some of these tags describe functions. I think it would be very useful to consumers of this library to not just parse the raw data, but also to convert that data in to a javascript function. But I have no (good) idea for an API for that either.

xi commented 5 years ago

I read up on ICC and the different versions (the version I implemented is ICC2 which is still in wide use, but it is from 2001 and two newer versions of the spec are available). I must say I am a bit discouraged to work on all of that.

I think this library without my additions is already very useful. My additions would widen the scope considerably. I am not sure this makes sense without providing much more functionality on top. Any idea what would need to be done to make this a complete feature but still limit the scope?

lovell commented 5 years ago

I like the idea of exposing the A2B and B2A tags as functions. In terms of this PR, shall we focus on exposing these tables as Buffers for now then think about how to extend the API later?

The unit tests are failing as the expected whitepoint values differ from actual, e.g.:

  1) Parse valid ICC profiles sRGB:
      AssertionError [ERR_ASSERTION]
   "whitepoint": [
-    0.964202880859375
+    0
     1
-    0.8249053955078125
+    0
   ]

It looks like http://www.color.org/probeprofile.xalter could be added to the test fixtures, which would then cover a lot of the new logic being introduced here.

At the moment a Buffer containing the whole ICC profile is required so everything is already in memory. Exposing a WritableStream-based approach could work, but let's look at this in a separate change.

xi commented 5 years ago

My project went another way and I don''t really need this feature anymore, so I won't have time to complete this. Feel free to close the PR or fill the missing parts as you like. Thanks for the feedback along the way!