iTwin / itwinjs-core

Monorepo for iTwin.js Library
https://www.itwinjs.org
MIT License
615 stars 210 forks source link

Allow ToolAdmin.pickCanvasDecoration for viewing tools #6670

Closed a-gagnon closed 6 months ago

a-gagnon commented 6 months ago

Is your feature request related to a problem? Please describe. I'm writing a tool to rotate the view based off the mouse position, but I also allow user to input the angle numerically. For that, I display the current angle in degrees using a canvas decoration. I want to be able to click on it to change the value, but there is some code in ToolAdmin that prevents a viewing tool from interacting with a canvas decoration, ie:

  private pickCanvasDecoration(ev: BeButtonEvent) {
    const vp = ev.viewport!;
    const decoration = (undefined === this.viewTool) ? vp.pickCanvasDecoration(ev.viewPoint) : undefined;
    this.setCanvasDecoration(vp, decoration, ev);
    return decoration;
  }

Because of that, onMouseButton, onMouseEnter, pick and the likes never get called. I had to subclass InputCollector instead of ViewTool to get it working in the meantime, but that approach has other issues. What's the reasoning behind this? Not all view tools are started in 'single shot' mode.

Describe the solution you'd like Add a function on the ViewTool, like canPickCanvasDecorations, that I could override and set to true whenever I'd like to interact with canvas decorations

Describe alternatives you've considered As said previously, I subclass InputCollector in the meantime, but it has its quirks. For example, if I start any ViewTool, it won't let me click start my tool since the viewing tool has priority over an input collector.

For reference, a screen cap of the tool: image

pmconne commented 6 months ago

@bbastings

bbastings commented 6 months ago

@a-gagnon @pmconne Is there some reason your ViewTool can't check it's own decoration? The LookAndMoveTool uses canvas decorations for touch input controls (like the thumbsticks) and just checks them itself.

The main reason this check exists is to prevent locating a decoration from the suspended PrimitiveTool or InputCollector when the view is changing (eats motion events, can show tooltip, etc.). We also didn't want tools like RotateViewTool to pick a decoration from the suspended PrimitiveTool when trying to identity the point to rotate about (ex. Measure tools measurements).

image

So no, not in favor of adding an option to ViewTool for "canPickDecorations" because of the issues mentioned and the fact that you really just want to pick your own decorations not all decorations...

a-gagnon commented 6 months ago

Thanks. I figured there was some reason.

It'll take some refactoring and expose some pieces I would've kept internal to the text editing class itself, but checking hit myself is a solution.