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

Use correct vector space definitions #64

Merged
merged 2 commits into from
Oct 28, 2014
Merged

Use correct vector space definitions #64

merged 2 commits into from
Oct 28, 2014

Conversation

jiahao
Copy link
Contributor

@jiahao jiahao commented Sep 29, 2014

  • Undefines errorneous +, * on LCHUv
  • Defines +, * on XYZ as a linear vector space
  • Defines +, * on all other ColorValues using intermediate arithmetic in
    XYZ space

cc @rennis250 @timholy

@timholy
Copy link
Contributor

timholy commented Sep 29, 2014

Images defines arithmetic on all AbstractRGBs, and I'm pretty sure it's going to stay that way. But for this package, this might indeed be the best thing to do. CC also @dcjones.

We also need to put some effort into increasing the efficiency of conversions. See discussion (not implementation) in #48.

@jiahao jiahao force-pushed the cjh/vectorspace branch 2 times, most recently from 0ee1833 to e3e02d8 Compare September 29, 2014 18:41
@rennis250
Copy link

Cool! LMS is also a linear vector space, so it is safe to define these operations for it, too, unless we want to keep XYZ as a common intermediary for these sorts of operations.

@rennis250
Copy link

EDIT: Ignore what was in this box here. I need sleep. Will hopefully have clearer thoughts after that...

@m-lohmann
Copy link
Contributor

CIECAM02 is not an “LMS system”, but a color appearance space, as you
already mentioned in your post.
The LMS space can be linearly transformed to XYZ and vice versa, there
are no nonlinearities involved.

CIECAM02 uses the linear CMCCAT2000 transform, but then several
nonlinear factors, like the involvement of viewing conditions,
adaptation and a nonlinear post-adaptation response compression are
involved, too.

On 29.09.2014 21:37, Robert J. Ennis wrote:

Oh wait, ignore that. I see that the intention is to implement the
CIECAM02 colour appearance model as the LMS system. As far as I can
tell, it is only partially implemented? If that is the case, then,
once fully implemented, that LMS model is not linear.


Reply to this email directly or view it on GitHub
#64 (comment).

@rennis250
Copy link

Ah, okay, I didn't know the name of the sole transform, only the full model. Thanks for the info!

I suppose this can be merged now?

@jiahao
Copy link
Contributor Author

jiahao commented Oct 5, 2014

I've added some tests. It looks like the Lab and LMS ColorValues behave like a linear vector space too (up to floating-point roundoff); does that look right?

- Defines +, * on XYZ as a linear vector space
- Defines +, * on all other ColorValues using intermediate arithmetic in
  XYZ space
@jiahao jiahao force-pushed the cjh/vectorspace branch 3 times, most recently from a1d85cc to 3c1d2fd Compare October 5, 2014 02:09
@coveralls
Copy link

Coverage Status

Coverage increased (+0.18%) when pulling ed449b4 on cjh/vectorspace into dcfc38f on master.

@rennis250
Copy link

Lab is also non-linear. It basically applies a cube root transform to each of the X, Y, and Z values. But, yes, LMS is a linear vector space.

jiahao added a commit that referenced this pull request Oct 28, 2014
Use correct vector space definitions
@jiahao jiahao merged commit 8b42b0a into master Oct 28, 2014
@jiahao jiahao deleted the cjh/vectorspace branch October 28, 2014 03:32
jiahao added a commit that referenced this pull request Feb 18, 2015
Fixes #59 again. This time the xyz_to_uv path was hit because of the nonlinear conversions through LCHuv exposed by #64.
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.

5 participants