search

adobe-photoshop / spaces-design

Adobe Photoshop Design Space
http://adobe-photoshop.github.io/
Other
852 stars 74 forks source link

Option copy of layer causes error "Too many layers in layer index" #1613

Open ktaki opened 9 years ago

ktaki commented 9 years ago

Create a doc and place a rectangle. With the select tool, drag the rectangle while pressing Option key to create a copy of the rectangle.

Action layers.getLayerOrder failed: Error: Too many layers in layer index at Record.LayerStructure.updateOrder

screen shot 2015-06-18 at 10 55 56 am

mcilroyc commented 9 years ago

git bisect, you my only friend. first bad commit: 9cdf11a356e524b406642ca5ebbd5e93b5db9435 I'll continue investigating...

ktaki commented 9 years ago

Verified the fix.

chadrolfs commented 9 years ago

Looks like we have a regression here. Ran into this duplicating Artboards

chadrolfs commented 9 years ago

screen shot 2015-09-29 at 2 37 11 pm

mcilroyc commented 9 years ago

When duplicating a pair of artboards, sometimes I can repro this error. My hunch is that it occurs when we get a "moveToArtboard" event before the "move" event. It can't reliably get it to happen. Any parting thoughts on this @volfied since you have some recent familiarity here, I think.

iwehrman commented 9 years ago

When I select two artboards (each with two rectangles inside) and option-drag copy them, I get: Verification of postcondition 1 failed for action layers.addLayers, which is _verifyLayerSelection. This might be spurious however (since we're processing the events out of sync with the action?) because the selection looks OK to me.

iwehrman commented 9 years ago

Other than that, I can't break it. So, besides some requests for additional in-line documentation and a question about a different debounced handler, I'm basically satisfied for the time being. Also, to reiterate our discussion off-line: what need for a longer-term solution is to articulate a core request for how all these notifications ought to work, or at least how they could be changed to be significantly less broken.

mcilroyc commented 9 years ago

@iwehrman I think your last two comments were supposed to be in the PR... but anyway: At first glance I do think the verifyLayerSelection failure is spurious and not easily avoidable, but I'll look more closely.

iwehrman commented 9 years ago

(Derp. Moved them over for later readability.)

iwehrman commented 9 years ago

FBNC to @ktaki. Please give this one a little extra love when verifying because some question remains about the correctness of the fix.

mcilroyc commented 9 years ago

Also @ktaki if you could please read through the description of the associated PR #2881 to see the related unreported issues that this fixes.

ktaki commented 9 years ago

I spent over an hour to reproduce this issue on Mac and Windows. I saw the error happened on Mac twice and none on Windows. I think this issue has not been fixed completely, but now it is much harder to reproduce than before. If we want to improve this fix after 16.1, we can keep open this and remove M3 for now. How do you think?

iwehrman commented 9 years ago

Bouncing back to @mcilroyc, reducing to Medium Pri since it is apparently harder to fix now.

mcilroyc commented 8 years ago

@ktaki - if you happen to run in to this again, please let me know. I can not repro.

ktaki commented 8 years ago

@mcilroyc - It is relatively easy now. Open new artboard doc. Select Artboard 1. Drag the artboard 1 while pressing option key. Repeat the option-copy two or three times. The error occurs. Tested on Mac.

mcilroyc commented 8 years ago

I just tried ~20 option-copy of artboards and did not see the error. Would you mind taking a screenshot of the console with some of the messages leading up to the error @ktaki ?

ktaki commented 8 years ago

Here it is.

screen shot 2016-01-14 at 1 15 46 pm

mcilroyc commented 8 years ago

Thanks @ktaki - I turned on event level logging and then was able to repro once. I think I figured out a quick fix. If you would like to pre-test the branch from this PR, feel free. https://github.com/adobe-photoshop/spaces-design/pull/3537

baaygun commented 8 years ago

@ktaki I see you can still break this if you try hard enough. But you also said it's probably ok since it's a marginal case. I leave it to your discretion.

ktaki commented 8 years ago

With 100ms delay inserted, it became a lot harder to reproduce the error on my MacBook Pro. It will take a medium effort to reproduce the error on slower Windows desktop, and it is still relatively easy to do it with Surface Pro 3. Does the 100 ms delay work? It depends on the platform and CPU speed.

ktaki commented 8 years ago

@mcilroyc Do you want to try increasing the delay or looking into some other solutions?

mcilroyc commented 8 years ago

I'll leave this open, and assigned to me with a medium priority. Hopefully we can find a better solution than just increasing the delay. For now I think 100ms is a good balance between optimizing happy-path performance for our target users and preventing errors on slow machines.