seqviz icon indicating copy to clipboard operation
seqviz copied to clipboard

implement one row linear viewer

Open bugzpodder opened this issue 2 years ago • 6 comments

fixes #179

https://codesandbox.io/s/dawn-haze-ey1k3v?file=/demo/package.json

bugzpodder avatar Apr 27 '23 01:04 bugzpodder

Seeing one small thing @bugzpodder where the annotation wrapping edges are showing up along the linear viewer like:

Screenshot from 2023-05-06 15-58-25

That should be fixable down in Annotations around among all this nonsense for edges and stuff: https://github.com/Lattice-Automation/seqviz/blob/develop/src/Linear/Annotations.tsx#L130-L182

jjti avatar May 06 '23 20:05 jjti

Looks a bit better but still not ideal. not too familiar with SVG but ideally can make the edge invisible or same as background color but couldn't really figure it out. Screenshot 2023-05-06 at 11 48 14 PM

bugzpodder avatar May 07 '23 06:05 bugzpodder

@bugzpodder I'm toying around with something like what's in this example PR's commit: https://github.com/Lattice-Automation/seqviz/pull/211/commits/95fa0620b810a0e1bc595a2f6d9468684223e7ed

We'd literally have only a single SeqBlock that covers the entire sequence.

Some pros/cons of that different approach in that commit above:

Pros:

  • can re-use existing InfiniteScroll (just one row)
  • we don't have to worry about jagged annotation edges/translation edges/etc. Lots of elements account for the edges of SeqBlocks, so all will have to have modified rendering if the oneRow case image
  • pretty minimal changes to existing linearProps and no need for the max* variables where we build up the maximum across all blocks (because there's only one block)

Cons:

  • new logic needed for scrolling left/right on selection
    • eg if we click base 1000 on circular, this should scroll the single row viewer to bp 1000 on the single row viewer
  • rendering is extremely slow. Selecting ranges of sequences is unacceptable slow, 3-5sec latency. We need to add in some logic to check for and avoid rendering anything outside the range of the .la-vz-seqblock-container (which I haven't done in that example commit)
    • I was thinking maybe we could put it in SeqBlock.findXAndWidth... like maybe it returns:
findXAndWidth(firstIndex: number, lastIndex: number): {width: number, x: number, render: boolean}

where render is false if the passed first/lastIndex are way outside the viewer's current viewer, and the corresponding component shouldn't bother rendering at all for performance reasons. For example, in Linear/Index.tsx, we'd skip rendering ticks if we see the ticks index is so far outside the Viewer that it'd never show up anyway like:

    return tickIndexes.map(p => {
      let { x: tickFromLeft, render } = findXAndWidth(p - 1, p - 1); // for midpoint
      if (!render) {
        return null;
      }
      tickFromLeft += charWidth / 2;

I'm open to other thoughts/opinions on this. And I realize it's a bit of a drift in approach from the PR, but I think it might be less total work...

jjti avatar May 07 '23 22:05 jjti

I think that's a really neat idea. If we can render using just a partial block and do some optimizations so the svg path are only drawn for whats in the viewport that would obviously be best.

definitely prefer that since its much easier to reason with and code.

bugzpodder avatar May 08 '23 03:05 bugzpodder

This is awesome. Looks like we've made some progress with this PR. Can we carry this over the finish line? @jjti @bugzpodder

leshane avatar Sep 16 '23 13:09 leshane

@leshane I think @jjti is planning a simpler implementation. The issue with my version is that when two blocks are connected together you see some weird artifacts: https://github.com/Lattice-Automation/seqviz/pull/205#issuecomment-1537557173

bugzpodder avatar Sep 17 '23 19:09 bugzpodder