adobe-photoshop / generator-core

Core Node.js library for Adobe Photoshop CC's Generator extensibility layer
MIT License
688 stars 97 forks source link

Incorrect indexRange on layersAdjusted property in imageChanged event #187

Open iwehrman opened 10 years ago

iwehrman commented 10 years ago

(I'm filing this here so I don't forget it and because I don't like Watson.)

Create a document with three layers: at index 0 and index 1 there are regular layers, and at index 2 there is an adjustment layer. Clip the adjustment layer to the the layer at index 1, resulting in the following docInfo: https://gist.github.com/iwehrman/22536785c1da8582a9d6). Then change the adjustment layer’s parameters, resulting in the following imageChanged event: https://gist.github.com/iwehrman/8b452c079458efc001ab

The “clipGroup” property there makes sense: it means that middle layer changed because it’s part of a clip group, which spans from index 1 to 2 (I.e., the top two layers), and also that the parameters of the adjustment layer itself changed. But the layersAdjusted property on the adjustment layer is wrong: it means that the range of layersAdjusted is index 0 through 1, but because the adjustment layer is clipped the range of layersAdjusted should really be only 1 through 2 (or possibly just 1, depending the exact semantics of layersAdjusted.) The range 0 through one would make sense if the adjustment layer were not clipped.

CC @timothynoel

iwehrman commented 10 years ago

Test PSD.

iwehrman commented 10 years ago

Assigning to @timothynoel for whenever he comes up for air.

timothynoel commented 10 years ago

My general philosophical view of imageChanged is that they should be viewed invalidations, and are allowed to be a little broad in the interest of speed. (like the bounds messages) In this particular case, we already know the clip group range for other reasons, and can address this without a performance penalty, but a caller should assume the messages aren't necessarily exact, but may become more precise in future versions. As an example, it wouldn't make sense to refine layersAdjusted even further to only include layers that overlap the actual change region of the adjustment layer/mask, as there would be a performance penalty in that case.