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

Delayed subtrees #137

Merged
merged 9 commits into from Nov 14, 2013

Conversation

Projects
None yet
2 participants
@byorgey
Member

byorgey commented Nov 13, 2013

Depends on diagrams/diagrams-core#47; that should be merged first.

This pull request has two major components:

  1. Fixes #112 by creating arrows as delayed leaves. We can only correctly draw an arrow once we know the final transformation applied to it. See diagrams/diagrams-core#47 for more details on how this is accomplished.
  2. While we are at it, use the fact that we know the global context at arrow creation time to take the default arrow head, tail, and shaft colors from the current line color.

Brent Yorgey added some commits Nov 13, 2013

Brent Yorgey
change arrow' to produce a delayed leaf.
The idea is that arrow' can now take the *final* transformation into
account so it can properly decide what to draw.  See #112.

Things seem to be almost working... but it seems we don't quite handle
frozen transforms properly yet, since things are flipped in the Y-axis.
Brent Yorgey
D.TwoD.Arrow: Use existing line color as default for tail, head, and …
…shaft

This is another cool thing enabled by doing arrows as delayed leaves:
in addition to knowing the overall transformation in effect when
drawing an arrow, we know the overall *style*!  So we can grab
e.g. the line color and use it as a fill color for the arrow head and
tail.  In particular, this patch sets things up so that line color is
used by default for the head, tail, and shaft of an arrow.  That is,
doing (arrow ... # lc blue) will result in a completely blue
arrow (including head and tail).  This can be overridden by setting
headStyle, tailStyle, or shaftStyle as usual.
@byorgey

This comment has been minimized.

Show comment
Hide comment
@byorgey

byorgey Nov 13, 2013

Member

Whoops, there's something screwy about the way envelopes are handled, please don't merge yet.

Member

byorgey commented Nov 13, 2013

Whoops, there's something screwy about the way envelopes are handled, please don't merge yet.

@bergey

This comment has been minimized.

Show comment
Hide comment
@bergey

bergey Nov 14, 2013

Member

What does the first occurrence of 'default' mean?

Member

bergey commented on src/Diagrams/TwoD/Arrow.hs in 1ba6733 Nov 14, 2013

What does the first occurrence of 'default' mean?

This comment has been minimized.

Show comment
Hide comment
@byorgey

byorgey Nov 14, 2013

Member

I have no idea. I will remove it.

Member

byorgey replied Nov 14, 2013

I have no idea. I will remove it.

@byorgey

This comment has been minimized.

Show comment
Hide comment
@byorgey

byorgey Nov 14, 2013

Member

Hah, it's not the envelopes that are screwy. The envelopes are exactly what they should be. It's the shafts---they are getting flipped somehow (not reversed, but mirrored along the axis of the arrow). No clue why yet.

Member

byorgey commented Nov 14, 2013

Hah, it's not the envelopes that are screwy. The envelopes are exactly what they should be. It's the shafts---they are getting flipped somehow (not reversed, but mirrored along the axis of the arrow). No clue why yet.

Brent Yorgey
D.TwoD.Arrow: bug fix: actually take unfrozen transform into account!
Previously, arrow' simply took the tranformation and applied it to the
points origin and (len,0), and then drew an arrow between the images
of those points.  However, the transformation might also be doing some
flipping or shearing which was getting ignored: affine transformations
are not determined by their action on two points.

This was causing, e.g. shafts to be drawn flipped along the axis of
the arrow with the SVG backend, since the SVG backend does a `reflectY`
as the last thing before freezing, but it was getting ignored.

This patch makes `arrow'` properly apply the transformation to the
shaft trail prior to uniformly scaling and positioning it.
@byorgey

This comment has been minimized.

Show comment
Hide comment
@byorgey

byorgey Nov 14, 2013

Member

That ought to do it. As far as I know this (along with diagrams/diagrams-core#47 and diagrams/diagrams-doc#40) are ready for merging. The website builds and all the example diagrams seem correct.

Member

byorgey commented Nov 14, 2013

That ought to do it. As far as I know this (along with diagrams/diagrams-core#47 and diagrams/diagrams-doc#40) are ready for merging. The website builds and all the example diagrams seem correct.

bergey added a commit that referenced this pull request Nov 14, 2013

@bergey bergey merged commit 7383eff into master Nov 14, 2013

1 check passed

default The Travis CI build passed
Details

@byorgey byorgey deleted the delayed-subtrees branch Nov 14, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment