search

adobe-photoshop / spaces-design

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

Fix unrecoverable state after failure in sampling LLSO #3815

Closed shaoshing closed 8 years ago

shaoshing commented 8 years ago

DS will fail to recover from action failure if the currently selected tool is sampler. This is because the SamplerOverlay is unable to read the selected layers in its shouldComponentUpdate function, as the current document is unavailable during resetting. This PR fixes the issue by checking the existence of the current document. I also checked the other tools to make sure doing the same checking in their SCU function.

Fix: https://jira.corp.adobe.com/browse/PS-2645 Mitigate: https://github.com/adobe-photoshop/spaces-design/issues/3108 (it should not be a ship blocker now)

iwehrman commented 8 years ago

Besides the nit, this looks correct to me. @chadrolfs or @ktaki: can you test this fix before we merge it?

chadrolfs commented 8 years ago

Sure, I'll take a look now.

chadrolfs commented 8 years ago

Still was able to run into the bug and get into the unresponsive state.

chadrolfs commented 8 years ago

The first time I sampled, undo, sampled again, it recovered, however if you do it a few more times, you'll get into that state. Here's the tail end of the errors (using compiled js)

screen shot 2016-04-26 at 11 40 34 am

shaoshing commented 8 years ago

Hmm I am unable to reproduce that state after re-sampling more than 10 times. @chadrolfs can you show me the detailed steps? Like which assets did you use for testing.

chadrolfs commented 8 years ago

Something is off. I am connected via devtools to this branch compiled build and unable to sample now because it's falling through to standard PS. When I try to hit Space to sample, it switched to Hand (standard PS behavior).

chadrolfs commented 8 years ago

I should mention, I'm also using mainline m.41 build

shaoshing commented 8 years ago

@chadrolfs thanks for the help, the problem is related to the debug menu. In production mode, we remove the DEBUG menu during initialization to hide it from the user. However, it will keep removing the last menu every time when DS is recovered, which eventually lead to an empty menu list that causes the unrecoverable state. I've pushed some change to address the issue. Please test again :D

chadrolfs commented 8 years ago

Ok, looks like the unrecoverable state is avoided now. The only other issue which we can defer and not worry about at this point is the bounds do not get reset correctly on undo, but once you change tools (to Select for example) it updates correctly.

ds-pinkbounds