...better than moving the whole object, let an orange handle go off the screen...
...and even more tracers can be seen if you make the orange handle go off the
screen when dragging a green handle...

Created attachment 45098 [details]
here is a beziergon tracer using the drag-green-handle-resize-while-orange-handle-goes-off-screen test case

i really like the beziergon....

a quick look at app/display.c:ddisplay_add_display_area() showed that the update
area is not calculated quite right when one of the orange handles goes off the
screen.  should be a quick fix...

i am extraordinarily enthusiastic about open source -- i am also new to it, in a
way...  

should i be sending updates on this thread, or just get it done, and send in the
patch?  

spasiba 8-|

tom 

Created attachment 45161 [details] [review]
patch to fix bug 300055

the beziergon needs its orange handles to be drawn to properly render the
control lines.	this patch removes two lines (probably there to improve
performance?) from display.c:ddisplay_add_update() to allow the orange handles
to be drawn even when they are above the visible rect --  the subsequent call
to rectangle_intersection(r, &ddisp->visible); should calm any performance
concerns...

I'm not sure this fix exactly makes sense.  It means that rendering outside the visible rect could 
cause extra rendering inside it.  The Right Way ought to be to make sure that the rendition of the 
control lines causes a display update.  Not that I think this patch would lead to big performance 
problems, but I'd rather have the right fix.

The right fix would appear to be in beziershape_draw_control_lines and 
bezier_conn_draw_control_lines, both of which should probably add an update area.

ja mon -

i totally agree and was being a bit lazy.  but that is the power of open source 
versus the broken commercial software development world of non-technical 
technology manager...

...ok i will stop waxing...  

i was thinking the same thing, bezier_conn_draw_control_lines -- but this might 
mean that i will actually understand how an object manages its series of rects 
that define its update areas!  oh no!

the underlying bug is that the bounding box for beziergon is not calculated
correctly when a handle of a control line goes off the visible area.  the
beziergon relies on both lib/boundingbox.c:polybezier_bbox() and
rectangle_union() in app/display.c:ddisplay_add_update() to get itself updated.

replacing the many lines of polybezier_bbox() with a simple loop like this:
for (i=1;i<numpoints;i++) {
  Point ctr1;
  Point ctr2;
  ctr1.y = MIN(pts[i].p2.y, MIN(pts[i].p1.y, pts[i].p3.y));
  ctr1.x = MIN(pts[i].p2.x, MIN(pts[i].p1.x, pts[i].p3.x));
  ctr2.y = MAX(pts[i].p2.y, MAX(pts[i].p1.y, pts[i].p3.y));
  ctr2.x = MAX(pts[i].p2.x, MAX(pts[i].p1.x, pts[i].p3.x));
  
  rectangle_add_point(rect,&ctr1);
  rectangle_add_point(rect,&ctr2);
}
...fixes the bug (the curve is always inside the 4 control points -- note the
loop starts at one to avoid the MOVE_TO), but now a function which was written
to calculate the minimum bounding box for a bezier curve has been specialized to
handle the beziergon (albeit better).

so i am thinking that i should i leave polybezier_bbox() alone and create a
separate function to handle the control lines.  thoughts? 

...i should mention that while the code above fixes the bbox for beziergon, it
does not do the same for polygon (the triangle).  

i guess the idea would be that either beziergon would get its own bbox function
in lib/boundingbox.c or we would try to work on the logic in app/display.c to
not write off the orange handles which do not intersect visible -- maybe the
association with the dashed control lines not unlike the "line" shape does when
its arrow goes out of visible... a third approach would be to work ymlisf (yet
more logic into a sinlge function) and rename polybezier_bbox to
polybeziercontrol_bbox() hehehehehe...... 

...i still think that it is strange that the dashed lines are not considered
objects to be updated -- they do intersect visible...  ...after looking at
"line" for some time, i came to understand that ddisplay_add_update_pixels is
called three times for one "line" update: once for each of three small rects
which are associated with either the two conn points or the mid point of the
"line" shape.  but then i realized that it is not a union of those three rects
(say, for example, in ddisplay_add_update()) which allows "line" to know that it
intersects visible when half of it does not, but rather its line_bbox function
and some gdk magic -- is that right?  

..one more thing -- i do realize that the Point declarations (above) go outside
the loop 8-| 

Created attachment 45424 [details] [review]
patch (2) to fix 300055

here is a patch that simply creates a very simple
boundingbox.c:beziergon_bbox() function to simply return an effective bbox for
beziergon to simply fix the bug 8-|

the only other change is to beziershape.c to call the new function instead of
what it used to call - boundingbox.c:polybezier_bbox().

nothing brilliant....  the concept would be that it is ok for some shapes to
create a special bbox function as needed.  

hpl

Created attachment 45427 [details] [review]
path to fix bug 300055

here is another version of the same patch (above) where
bezier_conn.c:bezierconn_update_boundingbox() also calls the new
beziergon_bbox() in the place of polybezier_bbox() which fixes the same bug in
Bezierline too...

have not heard anything on this...  you have life?  wife?

...possibly a different decomposition is preferable?  ....I can think of three:

1) shape knows its bounding box -- this would translate into a superclass 
method that can be overidden in the shape subclass -- all shape objects have a 
member method that can return/reference its bounding box.

2) keep on track and make a small edit to the polybezier_bbox() func to account 
for control lines on a bezier-curve-shape (both Beziergon and Bezier line need 
this).

3) the change i submitted: objects may use an existing boundingbox.c function 
or add one of their own.

how it sound you?

tomas

I do indeed have both life & wife, and wife's aunt's visit from the US has made
my life infringe on my Dia time.  Sorry to have left you going off on a tangent.

Problem with using BBox is that it's really overloaded to mean both "the area
that needs to be redrawn" and "how big the object is".  The two are not the same
for beziers, as the control lines are only rendered when it is selected.  The
latter is for instance used to determine the borders of the printed page.  I've
considered splitting the bbox functionality into two.  But that is not for now.

The polygon does not have this problem, as it has no control lines.

Comment #8:  No, that would make the BBox larger than the actual (non-selected)
rendering, leading to strange page sizes.

Comment #13: Huh?

Newest patch: Apart from the BB issues I mention above, it also ignores the
extra spacing required for arrowheads.

Can you see any reason my comment #6 would not fix it?

wife, life, dia ...and it was good ;-)  --me too...

question: when is the DrawFunc bezier.c:bezierline_draw() called?  this appears
to be the only reference to bezierconn_draw_control_lines()...

i am working on "add an update area" <a
href="http://bugzilla.gnome.org/show_bug.cgi?id=300055#c6">comment #6</a> above.

until i understand where this call comes/does not come into the stack, i am
focusing on beziershape.c:beziershape_draw_control_lines() 

ps - how do you add those links/anchors in the bugzilla comments?  clearly mine
(see #15) was not quite right...

oh, one more question -- when you get a patch (cvs diff std out) which you want
to add to the tree, what command to you use to add it?  is there another cvs
command to roll it back?   ...thanks!

To me comment #6 sounds like a recipe for an endless recursion.
[1] Object is requested to draw itself ->
-> object invalidates itself -> [1]

But I don't know more about the issue and am only here to make
http://bugzilla.gnome.org/reports/patch-report.cgi?product=dia&min_days=7&max_days=180
look better :-)

ja mon - 

fyi - i am moving (house) as we speak so my linux (fc3) pc is packed -- and 
bazaar twist -- i am closing on a house -- but the house had a water pipe burst 
with significant damage to the flooring two days ago -- blah, blah, blah, 
insurance, blah, blah.. ...seller's policy is still covering the house...

i have a couple ideas that are still in 'gdb' mode.  ...even my notes on the 
fix are packed -- but i might unpack given that i might not be moving in to the 
new house for two weeks or more...  ...next couple days will tell...

...what do you think about the last patch i submitted on this?

thomas

Comment #16: The 'patch' command takes the diff output and applies it.  patch -R
reverses it, or you can remove the affected files and do cvs update.

I commented patch #45427 in comment #14.

The links appear automatically when you have keyword #number, e.g. bug #300055,
comment #19 etc.

Best of luck with the moving, it's always a hassle.

oh my goodness, first home is castle or black hole?  my hands are a bit numb
from the jackhammer :-)  fyi, i have been dia'ing all of this old house
(structure, elevation, plumbing, electrical) and loving it!  dia jpeg's of load
bearing structural elements on an exterior wall will go to the structural
engineer this weekend for new load and sheer calculations resulting from
doubling the size of a patio door...

Hey, that's a great use of Dia.  I did some floor planning for our new
apartment, too, but nothing that I would send to an engineer.  I bet you wanted
a line that went |------ 2.5m ------| for that, yes?  I should put up a TWiki
zone for examples... there, if you feel like uploading it, go to
http://faemalia.org/wiki/view/Technical/DiaExamples (create an account if
needed) and attach a PNG or two.

Created attachment 54612 [details]
mydia 1

Created attachment 54613 [details]
mydia2 

faemalia.com has an issue with new user reg.  when i hear from them, i will
post :-)

2008-05-19  Hans Breuer  <hans@breuer.org>

	[finally use dia_object_get_enclosing_box() to ... ]
	* app/commands.c : ... have bezier control points in field of view 
	when "show all" with selected bezier objects
	* app/object_ops.c ... get rid of bezier control line traces, because
	they are not included with the bounding box. Fixes bug #300055
	* objects/standard/bezier.c objects/standard/beziergon.c : calculate
	the enclosing box as control points plus bounding box