Prevent TouchCursor from being cleared while it is still needed. #1477
Conversation
Fix an issue with mobile devices where the TouchCursor would disappear for no apparent reason. It it due to the order in which we call enableSnap/enableLocate. Previously, only enableSnap would determine the presence of the TouchCursor. Now, both enableSnap and enableLocate do (see iTwin#1038). The following sequence will lead to the problem: ToolSettings.enableVirtualCursorForLocate = true; accuSnap.enableLocate(true); accuSnap.enableSnap(false); // some touch operation to turn the TouchCursor on (eg. onTouchTap) // maybe pick an element or something within the tool accuSnap.enableLocate(false); // this will clear the TouchCursor accuSnap.enableSnap(true); // we still want the TouchCursor, but user will have to perform another touch operation to put it back on Note: tools that directly call enableSnap/enableLocate might still experience this problem. Fixing this problem entirely would require AccuSnap to wait some time (1 frame?) before effectively clearing the TouchCursor.
|
@kabentley Could you please have a look? I need to provide updates once in a while for a fix that depends on this one. |
|
@a-gagnon can you please run |
Head branch was pushed to by a user without write access
a-gagnon
requested review from
amikoliunas,
Animagne,
bsteinbk and
calebmshafer
as code owners
a-gagnon
requested review from
ColinKerr,
diegoalexdiaz,
EarlinLutz,
mdastous-bentley,
mgooding,
paulius-valiunas,
ramanujam-raman-zz,
RBBentley,
scsewall,
swwilson-bsi,
wgoehrig and
a team
as code owners
|
@calebmshafer Done, but it looks like every reviewer from every repo needs to accept changes because of the |
@calebmshafer is that because he's running it on a fork? |
@pmconne yes it looks like Edit: Validated that this was the issue and opened PR #1637 to help avoid anyone else from hitting this. Going to merge the PR as is because the extra changelogs really aren't hurting anything... |
Fix an issue with mobile devices where the TouchCursor would disappear for no apparent reason. It it due to the order in which we call enableSnap/enableLocate. Previously, only enableSnap would determine the presence of the TouchCursor. Now, both enableSnap and enableLocate do (see #1038).
The following sequence will lead to the problem:
ToolSettings.enableVirtualCursorForLocate = true;
// Tool starts. isElementSelected = false
accuSnap.enableLocate(!isElementSelected); // on
accuSnap.enableSnap(isElementSelected); // off
// User is prompted to tap to invoke the TouchCursor
user taps and invokes TouchCursor
user taps and selects an element
accuSnap.enableLocate(!isElementSelected); // off. Clears the TouchCursor.
accuSnap.enableSnap(isElementSelected); // on.
// User is prompted to tap to invoke the TouchCursor
Note: tools that directly call enableSnap/enableLocate might still experience this problem. Fixing this problem entirely would require AccuSnap to wait some time (1 frame?) before effectively clearing the TouchCursor.