From: sfeam <sf...@us...> - 2017-11-12 22:07:43
|
On Sunday, 12 November 2017 13:24:28 Dima Kogan wrote: > sfeam <sf...@us...> writes: > > > That allows me to test before/after applying 0005 so that I can evaluate > > the intended change by itself. Here's what I see. > > > > 1) Something has gone wrong with the arrowhead direction. > > Try running arrowstyle.dem. All the arrows at the page bottom have > > inverted arrowheads. > > > > 2) The patch has lost the distinction between "fixed" arrowhead size and > > the default variable arrowhead size. Many of the arrows in the > > arrowstyle demo look strange because their heads are too big. > > > > I attach a revised version of your patch 0005 that addresses 1 + 2. > > > > It adds a parameter to draw_clip_arrow that passes the fixed/variable > > arrowhead size state. I changed most the call sites to match. > > This change is in patch 0006 > > Hold on. I don't understand this. What is "fixed"? The docs say ONLY > > By default the size of the arrow head is reduced for very short > arrows. This can be disabled using the `fixed` keyword after the > `size` command. > > I don't know what that means. "fixed" means "fixed arrowhead size". The arrow head is always the same size regardless of the vector length. Without this keyword the arrowhead size for short vectors is scaled down in proportion with the length. > A bit of testing with the demo you > mentioned revealed a few bugs with my draw_clip_arrow(). The best one I > have is attached. Note that it doesn't have "fixed" path since I can't > tell what it's supposed to do. If nothing else, the way you implemented > that path suffers from the truncation issue. That's not "suffering". That's the intended behaviour :-) By default the arrowhead scales with the vector length, so as the length goes to zero the size of the head does also. I don't think truncation is relevant. > With that function, the arrowstyle.dem demo looks "correct", except the > line-through-should-be-empty arrowhead problem you mentioned. Some > arrowheads are shorter than I'd expect, but adding "fixed" to the > arrowstyle in the demo makes it consistent. Is this the "fixed" you're > talking about? Yes, but the new code has to handle both the default case and the "fixed size" case. Adding "fixed" to the demo hides the problem. With regard to the code you attached, I think this part can never happen: bool drawbody = true; if( head < 0 ) { drawbody = false; head = -head; } Unless I overlooked a code path, draw_clip_arrow() is never called with (head < 0). That convention "negative means only draw the arrowhead" is used only by the term->arrow() entry, and draw_clip_arrow itself is the only remaining caller of term->arrow. So I think the code I sent previously in 0005-EAM-draw_clip_arrow-with-double-precision.patch was correct. |