Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SVG Export Improvements #303

Open
2 of 4 tasks
ElDeveloper opened this issue Aug 5, 2020 · 4 comments
Open
2 of 4 tasks

SVG Export Improvements #303

ElDeveloper opened this issue Aug 5, 2020 · 4 comments
Assignees
Labels

Comments

@ElDeveloper
Copy link
Member

ElDeveloper commented Aug 5, 2020

Anything else you can think of @sjanssen2?

@ElDeveloper ElDeveloper added this to the Second Beta Release milestone Aug 5, 2020
@sjanssen2
Copy link
Collaborator

  1. we might want to think about the dimension and position of the legends. They are currently positioned at (0,0) and have a fixed size (relative to one hard coded variable). It might be more elegant to re-size according to drawing area.
  2. we discussed to replace the 15 line arc by one real arc for the circular layout, however, this would mean some extra math that can be messed up + intelligent traversal to differentiate between arc lines and "normal" edges
  3. shall we aim to write node names into SVG tags or would that bloat the file too much?

@ElDeveloper
Copy link
Member Author

Regarding labeling nodes, maybe we would want to think about showing labels for some nodes and just displaying those instead. But that sounds like something outside of the scope of SVG export 😄

@fedarko
Copy link
Collaborator

fedarko commented Aug 15, 2020

a few things we should fix at some point:

  • Rather than checking node color to determine whether or not to thicken the line of certain nodes, we should check the isColored property of the corresponding node. This is because it's totally possible for a node to get assigned the exact default color but still be technically "colored," and therefore drawn in the Empress UI with extra thickness (I doubt this is the case with any of the current color maps, but it's still a possibility ...)

    • I understand that currently exportSvg() works on a line-by-line basis (so it doesn't "know" which lines are associated with which nodes), but it should be possible to make this work by replicating some of the work done in getCoords() (and/or having getCoords() return some sort of structure mapping nodes to line positions in coords or something).
  • Line width scaling changed with Validate input line widths; scale widths based on number of leaves in the tree #275, and I forgot to update the SVG export code (sorry). We should fix this -- although a challenge is that the SVG stroke width isn't really comparable to the line widths the code currently uses (i.e. for us the default line width is now 0, but setting a stroke width of 0 will probably cause nothing to be drawn).

Since both of these seem like fairly annoying issues to resolve, it may be reasonable to just not alter line thickness when exporting SVG images of the tree for the time being.

@fedarko fedarko self-assigned this Aug 17, 2020
fedarko added a commit to fedarko/empress that referenced this issue Aug 24, 2020
@fedarko
Copy link
Collaborator

fedarko commented Sep 22, 2020

  • Configurable dimensions in the exported image (e.g. fit to letter paper)
  • Figure out why text styles are not applied to legend SVG in GIMP

kwcantrell pushed a commit that referenced this issue Sep 22, 2020
* MNT: SVG export code tweaks

- Declare variables with "var"
- exportSvg() -> exportSVG()
- exportSVG_legend() -> exportSVGLegend()
- other minor things

* MNT/ENH: tidy export code; mv legends to viz right

Progress on #303

* STY: rm redundant declaration

* MNT/DOC: document legend svg; consistent var names

* MNT: don't draw a black circle @ root anymore

* MNT: remove apparently redundant dom arg in func

* init vb text method skeleton

* MNT/TST: Add "legendType" attr to Legend; test

will be useful when representing arbitrary legends in svg export

* MNT: Store main legend in Empress, not SidePanel

* TST: add legend-main to test index.html

should unbreak tests

* STY: pret

* MNT/TST: store title easily in legend obj & test

again, will help svg export

* MNT: Delegate legend SVG exporting to Legend

Still gotta, like, test the bulk of this tho. also code needs
formatting

* MNT/STY: make legend export work; prettify

Also changed exported legend appearance a bit (no rounded rects,
bg is white, color squares have a black border, etc)

* STY: pret

* MNT: refactor svg code;attempt to fix viewbox junk

legend still isn't properly accounted for in the viewbox; gotta
fix. code is a lot prettier tho

* STY: jshint thing

* MNT: use consistent legend font in svg; fix width?

the width is a sledgehammer solution; need to make it exactly
correct. idk whats wrong

* BUG: fix (most of the) viewbox pbms with svg

Now legends are fit in to the viewbox, and things seem mostly ok.

Turns out the problem was using tree width (and hgt, but the problems
i was seeing were just from width b/c legend wasn't fitting in on
the x-axis) alongside "max X" stuff. Just storing things in units
of "max X", etc. and THEN doing the -> width conversion after
computing the SVG of the tree and legends fixed things.

Remaining problems:

- Push down the legend a bit; it looks like there is like a pixel
  or two at the top of it (in the border) that isn't shown.
- Remove extra blank space on the right edge of the legend. Likely
  due to (2 * 4) (aka 2 * NODE_RADIUS) stuff being overzealous in
  how I'm using it.
- Remove extra blank space on bottom edge of the legend when it is
  taller than the tree svg (e.g. for moving pictures f.m. coloring
  on level 7 taxonomy). Likely for same reason as above.
- The tree is still flipped upside down. wat.

Also, I need to document the heck out of the new SVG functions (in
both empress.js and legend.js), and add tests preferably. Then
it's PR time?

* BUG???: Negate y-coords in SVG export: fix #334

I have NO IDEA why this works. Maybe ... is Empress itself flipping
the y-coordinates in the first place, and the SVG export was right
all along??? argh.

* STY: pret

* ENH: UI mockup of PNG export/export inclusion opts

Inclusion options discussed with @ElDeveloper this morning.

For an initial PR i'll probably comment out a lot of this stuff
(e.g. the barplots) but it should be adaptable as we continue to
support more exporting stuff

* ENH: more ui mockup stuff for exporting

* MNT: remove unused read() func + skbio imports

we still need to include skbio as a dep because _plot needs to
reference skbio.OrdinationResult or something (and also some of
the python tests load trees as skbio TreeNodes before converting them
to bp), but none of the main Empress python code touches TreeNode now
at least

* ENH: by dflt don't draw circles; draw forall nodes

even for nodes without a specified name.

Closes #349 and closes #348.

TODO: improve selection menu interface for no-name nodes

* ENH: Polish UI for unnamed nodes

- Add fancy description in selected node menu, rather than saying
  "Name: null"
- Add note to BPTree.getNodesWithName() about dangers of having
  null as a key in BPTree._nameToNodes. Would be nice to test this
  case eventuallly ...

* STY: prettify

* DOC: clarify getNodeCoords docs

* TST: unbreak getNodeCoords test for #348

* ENH: add early PNG export (close #330)

Also comment out some unused stuff from the export panel for now

* ENH: keep sel node shape as circle; node rad attrs

See comments -- basically, when node circles were turned off,
the selected node shape turned into a square. now it's always a circle
which is nice ux-wise

* ENH: p major refactor of svg stuff

-Splitting up tree and legend exporting, per convo with @ElDeveloper
-Make tree export (SVG and PNG) work nicely (SVG still needs barplot
 and collapsed clade support; that'll come soon!)
-Add docs

Just left is legend exporting, which shouldn't be too bad. knock
on wood. That'll also include barplot legends, I think!

* STY: prettify, and abstract rgb code to new func

* ENH: polish up legend svg export + clean code

fixed bug where all legends getting plomped on top of each other
b/c maxy wasn't updated properly.

still has slight bug where there's extra empty space below the
bottom legend. not sure where originator is but it seems to be
exactly 2*unit pixels

* MNT: clean up leg exporting code a tiny bit

* MNT: for now, don't export the same legend 3 times

(was doing that to test multi legend junk but for now it's ok)

* BUG: Fix extra v-space bug (kinda) in leg export

* STY/DOC: prettify and upd8 comment re vspace thing

* MNT: whoops only include one of the tree legend

* DOC: document a lotta stuff

* DOC: add note re negating

* DOC: fix wording

* DOC: document exporting fixes

* DOC: word unnamed node message better

* DOC: close a paren and add context to null thing

* DOC: tidy up some comments

* ENH: center legend titles in export

* ENH: make legend colors snug, like in-app legends

So, part of my confusion earlier was that rects (i.e. color
squares) have their x and y describe their top left, but BY DEFAULT
text tags in SVG have their x and y describe their bottom left.
And you gotta use dominant-baseline to change that. Not sure what
sadist approved that but ok

* BUG/DOC: fix legend svg width;add docs;abstr code

* STY: prettify~~~

* DOC: document funky legend stuff

tbh i'm still kinda lost on why this works as well as it does but
OH WELL :D

* MNT: use SVG styles to cut down on redundant code

* STY: prettify the styling

i warned you about styling bro / i told you dog

* MNT: Create context in Legend.exportSVG()

removes some work from empress.js, and lets us adjust context
based on legend font (shouldn't be dynamic but just in case :O)

* DOC: font family -> font style

more accurate description

* MNT: Move exporting code to export-util.js

* STY: preeettify

* TST: unbreak JS tests (point to export-util.js)

Also rm'd an extra log statement

* DOC: Point Legend.exportSVG docs to ExportUtil

due to code reorganization in PR

* BUG: fix Emperor selection callback

Noticed that double-clicking on a category didn't work with the
biplot generated for this PR. turns out that that's b/c I forgot
to update the way the legend is cleared in that file. should be
good now

* PERF: Clear node circle buffer with toggling

Pair-programmed with @kwcantrell

* BUG/MNT: rename drawer radii to diams & upd8 svg
fedarko added a commit to fedarko/empress that referenced this issue Oct 8, 2020
Actually not that bad. wack.
fedarko added a commit to fedarko/empress that referenced this issue Oct 9, 2020
fedarko added a commit to fedarko/empress that referenced this issue Oct 10, 2020
kwcantrell pushed a commit that referenced this issue Nov 5, 2020
…ength legends; export collapsed clade shapes and barplots (#392)

* ENH: support collecting+exporting barplot legends

* BUG: Fix legend SVG title centering; <g> => <svg>

Also added a newline after </style>

* BUG: Improve legend SVG for SVG importers

-Use "long form" font specification for the style SVG (fixes problems
 with GIMP and Inkscape)
-Put the style code higher up in the output legend SVG -- has the
 effect of applying the rect stroke to the topmost legend rect, which
 was a problem in Inkscape but not in chromium (:thonk:)
-Add note about GIMP choking on dominant-baseline (tldr not worth
 worrying abt now i think)

* STY: reorder/split up text and .title svg styles

(the text styles are now across multiple lines)

* MNT: hang on to continuous props in Legend for SVG

similar to what we do for categorical export

* ENH: add newlines to within gradient svgs

make life less painful (tm)

* MNT: abstract some code within legend export; grad svg work

* MNT: Refactor colorer/legend handling of gradients

Now, things are split up into a "Solo" and "HTML" SVG -- the gradient
shown on the page is a combo of these, and the one we export is just
the Solo one. This makes scaling it properly for the export SO MUCH
EASIER AHH

All we gotta do now is just add in value text and update rowsUsed /
maxLineWidth. think that should be good?

oh also this is gonna explode the tests ofc. that is a job for TOMORROW
MARCUS (tm)

* ENH: Finish? gradient legend SVG exporting!

IT WORKS SO WELL AOGIHDSOIGHJ

something worth noting: there seems to be some unaccounted-for
horizontal padding on the right side in the cat legend export.
i matched it in the continuous legend export b/c it looks nice but
worth looking into...?

* MNT: add explicit padding to right side of cat SVG

it looks like things are the opposite from how i thought -- looks like
the perceived extra space was just due to the boldfont used in
estimating the texts (when you make the text bold it's almost snug
with the border on the right side). may as well add the same padding
as for the continuous legends so things look consistent ish.

Still, this leaves it kinda unclear as to why continuous legends were
so comparatively snug with the border until i added padding ... maybe
boldface numbers are just not that bigger? idk

UPDATE: yeah i checked it and bold numbers are basically the same size
but bold letters are much larger. mystery solved :100:

* ENH: initial support for exporting length legends

looks not great (gotta align max and min headers like in table)
but good enough tm

* ENH: make exported length legend look purdy

* STY: pret

* ENH: export collapsed clade shapes! #303

Actually not that bad. wack.

* ENH: draw full rectangle for unrooted collapsing

removes line in the middle

* ENH/STY: use stroke on svg triangles; prettify

* MNT: also use stroke for unrooted clade export

* DOC: document todos for clade collapsing export

* ENH: support exporting barplots in SVG!

Need to use paths for circular barplot curves, and maybe better
stuff for rect barplots. but it works :D

* Update readme re: #303 fixes :D

* PERF: Only specify stroke width for thick lines

since default is 1

* MNT: apply shape-rendering to SVG

shoutouts to https://stackoverflow.com/a/53309814/10730311.

this is very easy to configure (just a line in the svg header),
so if users prefer different things we can document this.

* MNT: make polygon exporting util func

* MNT: attempt to fix bounding box stuff

export is broken (height is somehow nan?) but at least this works now

* STY: prettify

* negate y coords and declare maxy in bb

still broken but much less so

* Space out adjacent polygon pts

fixes things! hey this works now

* PERF/BUG: don't even draw 0-length barplots

previously they were showing up in the SVG, probs due to precision
pbms. should beo k now

* STY: prett

* DOC: remove svg export disclaimer :D #303

* BUG: Fix #421

still gotta test, tho, which will likely need to be deferred until
after fixing the other pbms with this pr and tests .______.

* TST: fix a gradient svg test

* specify combined svg

* TST: split up test ref svgs by solo/html

* TST: Fix most of the colorer tests

* MNT: redo public attr stuff with a func approach

is safer -- delegates checking that colorer is continuous to, well,
colorer.

* TST: unbreak colorer tests!

* Say missing and/or not numeric in warning

* TST: unbreak legend tests :D

* STY: prettify tests

* MNT: Drastically simplify legend exporting

At least for cat legends -- no longer do we have to worry about
rows and units and all that. each legend just returns its width and
height, and that's all we care about.

* STY: pret

* UNBREAK CONTINUOUS LEGENDS :D

* Fix length legends

* ENH: show missing/nonnumeric warn in grad legends

* DOC: fix some docs stuff

* fix stats format

* populateTreeStats fixes

* STY: whoops

* MNT: Move radius bb expansion to sep function

addresses a comment from @kwcantrell

* PERF: don't cache barplot buffer

addresses @kwcantrell comment. Required breaking up the barplot
drawing stuff into a separate function that returns the coords

* BUG: fix reversed continuous color map legends

tldr gotta reverse the stop colors. reason this worked _before_ was
that it was accessing the interpolator to build up the stop colors

* simplify grad svg code a bit

* TST: test reverse color gradient bug is fixed

* STY

* MNT: Simplify gradient ID / suffix stuff

Addresses @kwcantrell comment.

* DOC: fix documentation stuff wrt #400
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants