Correctly handle 'view sel' for custom model with only some positions selected

[Tristan Croll]

The following bug report has been submitted:
Platform:        Windows-10-10.0.22621
ChimeraX Version: 1.6.dev202212302205 (2022-12-30 22:05:38 UTC)
Description
API question: is it a wrong move for me to use `Model.add_drawing()` to add a `Model` instance rather than a base-class `Drawing`? It *appears* to work OK, but I don't want to accidentally create any weirdness down the track.

Background: I was debugging an issue in ISOLDE's clashes widget where clicking on some entries would correctly focus the view on the clashing atoms, and others would zoom out to cover the whole model. Realised it was only happening when at least one atom was a CA, and narrowed it down to the Ramachandran markup. The reason there is that the `RamaAnnotator` model doesn't directly draw anything itself - that was being done by two subordinate `Drawing` instances, one for the CA beads, one for the cis/twisted peptide markup. 

When a CA atom is selected, the selection highlighting is echoed to the corresponding bead. That caused `RamaAnnotator.selected` to return True, but `RamaAnnotator.selected_positions` was returning None - so the "sel" selector ended up capturing the entire model. 

Changing the individual drawings to `Model` instances fixes that issue. To be completely honest I simply forgot to replace the `add_drawing()` calls with `session.models.add()`... but now I'm thinking it would be nice to keep it that way to avoid adding even more complexity to the model trees created by ISOLDE.

OpenGL version: 3.3.0 NVIDIA 528.24
OpenGL renderer: NVIDIA GeForce RTX 3070 Laptop GPU/PCIe/SSE2
OpenGL vendor: NVIDIA Corporation

Python: 3.9.11
Locale: en_GB.cp1252
Qt version: PyQt6 6.4.0, Qt 6.4.0
Qt runtime version: 6.4.1
Qt platform: windows

Manufacturer: HP
Model: HP ZBook Studio 15.6 inch G8 Mobile Workstation PC
OS: Microsoft Windows 11 Pro (Build 22621)
Memory: 34,007,068,672
MaxProcessMemory: 137,438,953,344
CPU: 16 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
OSLanguage: en-GB

Installed Packages:
    alabaster: 0.7.12
    appdirs: 1.4.4
    asttokens: 2.2.1
    Babel: 2.11.0
    backcall: 0.2.0
    blockdiag: 3.0.0
    build: 0.8.0
    certifi: 2022.12.7
    cftime: 1.6.2
    charset-normalizer: 2.1.1
    ChimeraX-AddCharge: 1.5.8
    ChimeraX-AddH: 2.2.3
    ChimeraX-AlignmentAlgorithms: 2.0
    ChimeraX-AlignmentHdrs: 3.3.1
    ChimeraX-AlignmentMatrices: 2.1
    ChimeraX-Alignments: 2.8
    ChimeraX-AlphaFold: 1.0
    ChimeraX-AltlocExplorer: 1.0.3
    ChimeraX-AmberInfo: 1.0
    ChimeraX-Arrays: 1.0
    ChimeraX-Atomic: 1.43.4
    ChimeraX-AtomicLibrary: 10.0
    ChimeraX-AtomSearch: 2.0.1
    ChimeraX-AxesPlanes: 2.3
    ChimeraX-BasicActions: 1.1.2
    ChimeraX-BILD: 1.0
    ChimeraX-BlastProtein: 2.1.2
    ChimeraX-BondRot: 2.0.1
    ChimeraX-BugReporter: 1.0.1
    ChimeraX-BuildStructure: 2.7.2
    ChimeraX-Bumps: 1.0
    ChimeraX-BundleBuilder: 1.2
    ChimeraX-ButtonPanel: 1.0.1
    ChimeraX-CageBuilder: 1.0.1
    ChimeraX-CellPack: 1.0
    ChimeraX-Centroids: 1.3.1
    ChimeraX-ChangeChains: 1.0.2
    ChimeraX-CheckWaters: 1.3.1
    ChimeraX-ChemGroup: 2.0
    ChimeraX-Clashes: 2.2.4
    ChimeraX-Clipper: 0.20.0
    ChimeraX-ColorActions: 1.0.3
    ChimeraX-ColorGlobe: 1.0
    ChimeraX-ColorKey: 1.5.2
    ChimeraX-CommandLine: 1.2.5
    ChimeraX-ConnectStructure: 2.0.1
    ChimeraX-Contacts: 1.0.1
    ChimeraX-Core: 1.6.dev202212302205
    ChimeraX-CoreFormats: 1.1
    ChimeraX-coulombic: 1.4.1
    ChimeraX-Crosslinks: 1.0
    ChimeraX-Crystal: 1.0
    ChimeraX-CrystalContacts: 1.0.1
    ChimeraX-DataFormats: 1.2.3
    ChimeraX-Dicom: 1.1
    ChimeraX-DistMonitor: 1.3
    ChimeraX-DockPrep: 1.1
    ChimeraX-Dssp: 2.0
    ChimeraX-EMDB-SFF: 1.0
    ChimeraX-ESMFold: 1.0
    ChimeraX-ExperimentalCommands: 1.0
    ChimeraX-FileHistory: 1.0.1
    ChimeraX-FunctionKey: 1.0.1
    ChimeraX-Geometry: 1.2
    ChimeraX-gltf: 1.0
    ChimeraX-Graphics: 1.1
    ChimeraX-Hbonds: 2.4
    ChimeraX-Help: 1.2.1
    ChimeraX-HKCage: 1.3
    ChimeraX-IHM: 1.1
    ChimeraX-ImageFormats: 1.2
    ChimeraX-IMOD: 1.0
    ChimeraX-IO: 1.0.1
    ChimeraX-ISOLDE: 1.6.dev0
    ChimeraX-ItemsInspection: 1.0.1
    ChimeraX-Label: 1.1.7
    ChimeraX-ListInfo: 1.1.1
    ChimeraX-Log: 1.1.5
    ChimeraX-LookingGlass: 1.1
    ChimeraX-Maestro: 1.8.2
    ChimeraX-Map: 1.1.3
    ChimeraX-MapData: 2.0
    ChimeraX-MapEraser: 1.0.1
    ChimeraX-MapFilter: 2.0.1
    ChimeraX-MapFit: 2.0
    ChimeraX-MapSeries: 2.1.1
    ChimeraX-Markers: 1.0.1
    ChimeraX-Mask: 1.0.1
    ChimeraX-MatchMaker: 2.0.11
    ChimeraX-MDcrds: 2.6
    ChimeraX-MedicalToolbar: 1.0.2
    ChimeraX-Meeting: 1.0.1
    ChimeraX-MLP: 1.1
    ChimeraX-mmCIF: 2.10
    ChimeraX-MMTF: 2.2
    ChimeraX-Modeller: 1.5.8
    ChimeraX-ModelPanel: 1.3.6
    ChimeraX-ModelSeries: 1.0.1
    ChimeraX-Mol2: 2.0
    ChimeraX-Mole: 1.0
    ChimeraX-Morph: 1.0.1
    ChimeraX-MouseModes: 1.2
    ChimeraX-Movie: 1.0
    ChimeraX-Neuron: 1.0
    ChimeraX-Nucleotides: 2.0.3
    ChimeraX-OpenCommand: 1.9.3
    ChimeraX-PDB: 2.6.10
    ChimeraX-PDBBio: 1.0
    ChimeraX-PDBLibrary: 1.0.2
    ChimeraX-PDBMatrices: 1.0
    ChimeraX-PickBlobs: 1.0.1
    ChimeraX-Positions: 1.0
    ChimeraX-PresetMgr: 1.1
    ChimeraX-PubChem: 2.1
    ChimeraX-ReadPbonds: 1.0.1
    ChimeraX-Registration: 1.1.1
    ChimeraX-RemoteControl: 1.0
    ChimeraX-RenderByAttr: 1.0
    ChimeraX-RenumberResidues: 1.1
    ChimeraX-ResidueFit: 1.0.1
    ChimeraX-RestServer: 1.1
    ChimeraX-RNALayout: 1.0
    ChimeraX-RotamerLibMgr: 3.0
    ChimeraX-RotamerLibsDunbrack: 2.0
    ChimeraX-RotamerLibsDynameomics: 2.0
    ChimeraX-RotamerLibsRichardson: 2.0
    ChimeraX-SaveCommand: 1.5.1
    ChimeraX-SchemeMgr: 1.0
    ChimeraX-SDF: 2.0.1
    ChimeraX-Segger: 1.0
    ChimeraX-Segment: 1.0
    ChimeraX-SelInspector: 1.0
    ChimeraX-SeqView: 2.8.1
    ChimeraX-Shape: 1.0.1
    ChimeraX-Shell: 1.0.1
    ChimeraX-Shortcuts: 1.1.1
    ChimeraX-ShowSequences: 1.0.1
    ChimeraX-SideView: 1.0.1
    ChimeraX-Smiles: 2.1
    ChimeraX-SmoothLines: 1.0
    ChimeraX-SpaceNavigator: 1.0
    ChimeraX-StdCommands: 1.10.1
    ChimeraX-STL: 1.0
    ChimeraX-Storm: 1.0
    ChimeraX-StructMeasure: 1.1.1
    ChimeraX-Struts: 1.0.1
    ChimeraX-Surface: 1.0
    ChimeraX-SwapAA: 2.0.1
    ChimeraX-SwapRes: 2.2
    ChimeraX-TapeMeasure: 1.0
    ChimeraX-Test: 1.0
    ChimeraX-Toolbar: 1.1.2
    ChimeraX-ToolshedUtils: 1.2.1
    ChimeraX-Topography: 1.0
    ChimeraX-Tug: 1.0.1
    ChimeraX-UI: 1.26
    ChimeraX-uniprot: 2.2.1
    ChimeraX-UnitCell: 1.0.1
    ChimeraX-ViewDockX: 1.1.6
    ChimeraX-VIPERdb: 1.0
    ChimeraX-Vive: 1.1
    ChimeraX-VolumeMenu: 1.0.1
    ChimeraX-VTK: 1.0
    ChimeraX-WavefrontOBJ: 1.0
    ChimeraX-WebCam: 1.0.1
    ChimeraX-WebServices: 1.1.1
    ChimeraX-Zone: 1.0.1
    colorama: 0.4.5
    comm: 0.1.2
    comtypes: 1.1.10
    contourpy: 1.0.6
    cxservices: 1.2
    cycler: 0.11.0
    Cython: 0.29.32
    debugpy: 1.6.4
    decorator: 5.1.1
    docutils: 0.19
    entrypoints: 0.4
    executing: 1.2.0
    filelock: 3.7.1
    fonttools: 4.38.0
    funcparserlib: 1.0.1
    grako: 3.16.5
    h5py: 3.7.0
    html2text: 2020.1.16
    idna: 3.4
    ihm: 0.35
    imagecodecs: 2022.9.26
    imagesize: 1.4.1
    importlib-metadata: 6.0.0
    ipykernel: 6.19.2
    ipython: 8.7.0
    ipython-genutils: 0.2.0
    jedi: 0.18.2
    Jinja2: 3.1.2
    jupyter-client: 7.4.8
    jupyter-core: 5.1.1
    kiwisolver: 1.4.4
    line-profiler: 3.5.1
    lxml: 4.9.1
    lz4: 4.0.2
    MarkupSafe: 2.1.1
    matplotlib: 3.6.2
    matplotlib-inline: 0.1.6
    msgpack: 1.0.4
    nest-asyncio: 1.5.6
    netCDF4: 1.6.0
    networkx: 2.8.8
    numexpr: 2.8.4
    numpy: 1.23.5
    openvr: 1.23.701
    packaging: 22.0
    ParmEd: 3.4.3
    parso: 0.8.3
    pep517: 0.13.0
    pickleshare: 0.7.5
    Pillow: 9.3.0
    pip: 22.2.2
    pkginfo: 1.8.3
    platformdirs: 2.6.2
    prompt-toolkit: 3.0.36
    psutil: 5.9.4
    pure-eval: 0.2.2
    pycollada: 0.7.2
    pydicom: 2.3.0
    Pygments: 2.12.0
    PyOpenGL: 3.1.5
    PyOpenGL-accelerate: 3.1.5
    pyparsing: 3.0.9
    PyQt6-commercial: 6.4.0
    PyQt6-Qt6: 6.4.1
    PyQt6-sip: 13.4.0
    PyQt6-WebEngine-commercial: 6.4.0
    PyQt6-WebEngine-Qt6: 6.4.1
    python-dateutil: 2.8.2
    pytz: 2022.7
    pywin32: 303
    pyzmq: 24.0.1
    qtconsole: 5.4.0
    QtPy: 2.3.0
    RandomWords: 0.4.0
    requests: 2.28.1
    scipy: 1.9.3
    setuptools: 65.1.1
    sfftk-rw: 0.7.2
    six: 1.16.0
    snowballstemmer: 2.2.0
    sortedcontainers: 2.4.0
    Sphinx: 5.1.1
    sphinx-autodoc-typehints: 1.19.1
    sphinxcontrib-applehelp: 1.0.2
    sphinxcontrib-blockdiag: 3.0.0
    sphinxcontrib-devhelp: 1.0.2
    sphinxcontrib-htmlhelp: 2.0.0
    sphinxcontrib-jsmath: 1.0.1
    sphinxcontrib-qthelp: 1.0.3
    sphinxcontrib-serializinghtml: 1.1.5
    stack-data: 0.6.2
    tables: 3.7.0
    tifffile: 2022.10.10
    tinyarray: 1.2.4
    tomli: 2.0.1
    tornado: 6.2
    traitlets: 5.8.0
    urllib3: 1.26.13
    wcwidth: 0.2.5
    webcolors: 1.12
    wheel: 0.37.1
    wheel-filename: 1.4.1
    WMI: 1.5.1
    zipp: 3.11.0

[Tristan Croll]

Heh - looks like I answered my own question (the answer is "no"). Causes model closing to fail (no error message - the model just doesn't close).

[Tristan Croll]

It's of course far from high priority, but I guess it would still be nice to have a supported way to keep these hidden in the Models panel while still playing nicely with respect to selections.

[Tom Goddard]

You came to the right conclusion. Adding a Model child using add_drawing() is going to cause problems. The distinction between Model and Drawing is that Model has an id number that can be used in commands, the Models in the scene all form a tree with parent child relations, Models have APIs like selection that Drawings do not have. Drawing is low level graphics with no id number, no notion of selection (but it can render an outline "highlight", the green outline).

So the basic problem is you want selection capabilities but you don't want the other things that go with Model (visibility in Models panel). We don't have support for that. Your use case sounds reasonable (selection but not a Model) but that is far too hard to retrofit in. In general, in this scenario I have made things a model. An example is in Chimera pseudobonds were not models, not shown in the model panel, but in ChimeraX they are models. This has nice consequences that a user can easily discover this and hide those pseudobonds through the Model panel.

[Tristan Croll]

I'm not sure what I'm doing wrong here, but this still isn't working the way I hoped. Here's the status of things:

RamaAnnotator is a model which doesn't do any drawing of its own. It delegates that to two sub-models - one for drawing the indicators for cis/twisted peptide bonds, one for the C-alpha spheres indicating residue Ramachandran scores. The latter is just an instanced sphere; for any selected C-alpha atom the corresponding position in this model is highlighted (otherwise the selection would be invisible since the sphere is bigger than the atom). If I call selected positions on the C-alpha drawing it gives the expected Boolean numpy array. But if I use the command "view sel" when any of the C-alpha spheres is highlighted, then the view zooms out to show *all* currently-visible C-alpha spheres. Could you offer any insight into what's going wrong?

[Tom Goddard]

Selection is a concept that only applies to Model instances, not Drawing instances.  The Drawings are the graphics rendering objects and have a notion of what part is "highlighted" with a green outline.  So if a Model like your RamaAnnotator uses Drawing children to actually do the drawing then the Model.get_selected() method needs to report if those Drawings are highlighted.  The default implementation of Model.get_selected() looks at child drawings and if they are highlighted it reports that the model is selected.

I'm not sure what you mean by "call selected positions on the C-alpha drawing".  Drawings don't have any method called selected_positions.  They have a highlighted_positions method.

The "view sel" command needs the bounds of the selected items.  The selection is obtained as an Objects instance (from chimerax.core.objects) and Objects only track models, atoms, bonds and pseudobonds.  So it is probably asking for the bounds of your RamaAnnotator model which is considered selected since some subpart is selected.  But because Objects has no capability to track the individual C-alpha spheres in your drawing submodel the selected objects Objects instance only knows that the entire RamaAnnotator is part of the selection and so its entire bounds are used.

So the basic issue is that there is no provision for representing arbitrary sub-objects of a model to be used in a command for instance via the "sel" specifier.  Only whole models, and atoms, bonds and pseudobonds are handled.