Have you bibisected who to blame/ask for the reason of these changes?

(In reply to Heiko Tietze from comment #1)
> Have you bibisected who to blame/ask for the reason of these changes?

No. IMHO the 7.0 behavior is the most correct. The 5.4.7 with closing the polygon but continuing the polyline seems totally wrong. Automatically filling the polygon, if you didn't select the filled polygon in 6.2.8 also seems wrong. I didn't check any versions before 5.4, so maybe there was even different behavior in earlier version. The user BAret in Ask also claims, there was even a recent change in behavior, but then I didn't test 6.3 or 6.4.

But after finding two different behaviors in the LOs last patchlevel releases and 7.0 master, I simply thought that's enough, as it already took a lot of time to test this. And bibisect seems pointless, if you don't know, what the correct behavior is. And "blindly" assuming any previous version is correct seems to be worse in this case.

I added Armin to the bug, as I know he is quite proficient in Draw, given he did some Draw usage talks at conferences.

Hi Guys,

I was inactive some time in-between, but I would have never seen a reason to change behavior. That interactive poly-creation is complicated anyways. Just want to make sure you have the whole picture:

- right mouse button: delete last pnt/undo/stepBach
- ctrl-click: close current poly, start new one (polyPoly topology)
- holding diverse qualifiers: hor/ver/diag/snap, etc...
- action to happen on mouse-up (important)
- lasso areas for hitTest/interactive editing/create/insert/delete points
- GluePoint/Marker stuff, esp. keyboard control for accessibility
- technically: interactive part is/was split to ImpSdrPathDragData due to also  being used in
  - contour dialog
  - edit-animation-path in impress animation effects along-path

So - PLEASE be VERY careful with doing changes here AT ALL ...
Changing the interaction handling is error prone and very sensible.
Of course because it was hand-crafted from the older days and never brought in good shape - sigh ---
Just my 2ct...

(In reply to Armin Le Grand from comment #3)

[A lot more stuff about various actions with polygon, which I didn't test]

> - ctrl-click: close current poly, start new one (polyPoly topology)

So I will state my questions again:

- What is the correct handling here from your POV?
- Would it make sense to split polygon and polyline for more consistent behavior?
- Is a PolyPolygon just a group of independent polygons, or do they have to share a common point, as 5.4 handling seems to somehow imply, as it continues the line?

Windows PolyPolygon function has: "The PolyPolygon function draws a series of closed polygons. [...] The polygons drawn by this function can overlap.", which doesn't exactly answer my last question either. But it suggests it makes sense split the polyline functionality, so you either draw polygons or polylines.

So from my POV and as I already stated, I think 7.0 is correct, so no fixes are needed. I'm still for the polygon / polyline split for UX consistency.

Hi Jan,

- What is the correct handling here from your POV?
AFAIK as it was before changes were made in that area. Obviously that touched stuff which was not on the radar - what always can happen, esp. in areas like these. Turn back, check was was intended to be changed - there may be good reasons - and do it in a way without side effects with knowing more usages/cases

- Would it make sense to split polygon and polyline for more consistent
behavior?

No. The one is filled, the other is not. Only consequent thing needed is to know the target type (currently in OBJ_TYPE AFAIK, argh, already had that better in aw080) and adapt each finished polygon in polyPolygon topology

- Is a PolyPolygon just a group of independent polygons, or do they have to
share a common point, as 5.4 handling seems to somehow imply, as it continues
the line?

Yes. No. :-)
Why should they share a point? This makes no sense at all - a polyPolygon is a bunch of single polygons. Topology defines adapting a FillRule to these when visualizing. We just have EvenOdd/NonZero where we use EvenOdd only - so it's more or less a xor - for each point # of being inside %1

Windows PolyPolygon function has: "The PolyPolygon function draws a series of
closed polygons. [...] The polygons drawn by this function can overlap.", which
doesn't exactly answer my last question either. But it suggests it makes sense
split the polyline functionality, so you either draw polygons or polylines.

So from my POV and as I already stated, I think 7.0 is correct, so no fixes are
needed. I'm still for the polygon / polyline split for UX consistency.

What difference do you see between creating polygons or polylines? Only diff is that one gets outline painted, other get fill painted. Anyways, for our attributation you can choose both as you wish. It is better  when the filled polygon is closed, but I would have to check if this is currently a criteria to offer fill in the attributes - there is also that polygon object type (OBJ_*)

Just tried - it definitely is an error that for polylines it gets closed when ending the interaction. Try to paint a 'u' shape with the interactive curve tool - you cannot due to it getting closed - why is that? You need to go to edit points and correct it to get your 'u' shape. For creating closed polygons there are the filled variations - if you want a closed but not filled poly, construct a filled, closed and change fill to non-filled - that's easier that finding the point tool and work on that poly.

That CTRL+ALT (was it just CTRL before?) behaviour is clearly buggy - when not using it the line variations (non-filled) are not closed - that's what it shopuld be.

Upps - sorry, was too fast. For the 'u' shape better take something with a at-leat two-stroke topology - 'E' or 'F' - where you have to paint two strokes. Using CTRL+ALT there currently closes the poly - that's definitely not wanted.
For filled ones this is wanted - try to paint an 'O' using two circles. It cannot really be seen if these get closed, but I think they get. That is correct.
So maybe the error is that for non-filled the detection to close finished part polygons is broken ant currently these get closed all the time...?

2nd note: the polygons in a PolyPolygon are completely independent. They do not need to have common real existing points where they intersect (that is the complicated part in the polygon clipping code when doing logic op's later, but for painting them this is not required at all).

Just had another idea - maybe someone tried to 'fix' this for the usage in Impress and the animation effects move-along-path. In that scaenario closing and even starting new at an existing point would make sense.
Of course you need to check/know that the path editing stuff is used in other places, too, to then e.g. add a 'close-all-and-start-at-a-common-point' and not break the other cases.
Just a guess...

Would have expected that polyPolygon has at least one common point. But there is no such statement on MSDN or AOO wiki and this picture (on the bottom) shows a use case: http://www.ucancode.net/faq/PolyPolygon-Polygon.htm

Hi Armin,

(In reply to Armin Le Grand from comment #5)
> - What is the correct handling here from your POV?
> AFAIK as it was before changes were made in that area. Obviously that
> touched stuff which was not on the radar - what always can happen, esp. in
> areas like these. Turn back, check was was intended to be changed - there
> may be good reasons - and do it in a way without side effects with knowing
> more usages/cases

Sorry, but that doesn't help me. As I already wrote 5.4, 6.2 and 7.0 behave different w.r.t. the polygon functionality. So I just checked out 3.3.0, which behaves like 5.4 ... most times.

Sometimes "Ctrl + Alt + left-click" close the polygon and continues with a line from the last manual point, sometimes the line doesn't appear and you can start a 2nd, independent polygon. I still think 7.0 is correct and therefore 3.3.0 is also broken.

> - Would it make sense to split polygon and polyline for more consistent
> behavior?
> 
> No. The one is filled, the other is not. Only consequent thing needed is to
> know the target type (currently in OBJ_TYPE AFAIK, argh, already had that
> better in aw080) and adapt each finished polygon in polyPolygon topology

There is especially the "polygon, filled" tool, so this is already handled separately by the GUI. I'm talking about GUI functionality, not code representation, where a polygon can always be closed or not. The latter case is then a polyline. And I'm just talking about splitting the tooling.

> - Is a PolyPolygon just a group of independent polygons, or do they have to
> share a common point, as 5.4 handling seems to somehow imply, as it continues
> the line?
> 
> Yes. No. :-)
> Why should they share a point? This makes no sense at all - a polyPolygon is
> a bunch of single polygons. Topology defines adapting a FillRule to these
> when visualizing. We just have EvenOdd/NonZero where we use EvenOdd only -
> so it's more or less a xor - for each point # of being inside %1

Great. This is also my understanding. I just wanted to make sure. 

> What difference do you see between creating polygons or polylines? Only diff
> is that one gets outline painted, other get fill painted. Anyways, for our
> attributation you can choose both as you wish. It is better  when the filled
> polygon is closed, but I would have to check if this is currently a criteria
> to offer fill in the attributes - there is also that polygon object type
> (OBJ_*)

From the UX POV, the "polygon" is not filled per default, but the "polygon, filled" is. Sure, you can easily change that via the area setting, but the whole "lines" tool box currently distinguishes between "normal" and filled shapes, even if the result can be easily switched later by changing the area attribute.

> That CTRL+ALT (was it just CTRL before?) behaviour is clearly buggy - when
> not using it the line variations (non-filled) are not closed - that's what
> it shopuld be.

It's like this also in 3.3.0. "Ctrl + Alt + left-click" closes the polygon here on Linux. Just "Ctrl + left-click" does nothing here.

FWIW I'm currently not planning to fix this. My only suggestion w.r.t. UX tooling is to split "polygon" and "polyline" functionality. So the first will always close the polygon and the latter never.

There is also the functionality "Close object" in the context menu. Interestingly I couldn't find a way to "Open" a closed object, eventually by removing a point.

If you edit the points, you can cut one, and the shape remains closed and the new line is drawn between the remaining points. If you paste that point now, you get two polygons overlapping from the old and new shape - interesting, unexpected result :-)

From the specification point of view, there are three possibilities:
<draw:polyline> A sequence of points, connected with straight lines, never closed, first point = last point does not make it closed.
<draw:polygon> A sequence of points, connected with straight lines, always closed, first point = last point is not needed to close be closed.
<draw:path> All from SVG 'path' element is allowed, but currently not all is implemented in LibreOffice.
<draw:polyline> cannot be filled.
<draw:polygon> can be filled.
<draw:path> Only those parts, which are closed by the 'closepath' command can be filled.
That is in not reflected in the UI of LibreOffice.

Problems in the UI:
1) Using Alt-Click to start a next sequence of points always inserts a 'closepath' command. But reading a shape without this 'closepath' command is possible.
2) Finish creating with a double-click on the first point, closes the shape automatically. But <draw:polyline> elements with same start and end point are possible and correctly read.
3) The naming in the UI uses 'Polyline' and 'Polygon' too, when it is a path-element with only straight lines.
4) The use of Alt-key with the 'Curves and Polygons'-tool is badly communitcated.

When starting with a polyline and using Alt-Double-click, it does only close the shape because of behavior 1).
Using Ctrl-Alt-Click is the same as only using Alt-Click. It closes the shape because of behavior 1) and goes into PolyPolygon-mode. The next line will belong to the same shape.

(In reply to Jan-Marek Glogowski from comment #11)

> There is also the functionality "Close object" in the context menu.
> Interestingly I couldn't find a way to "Open" a closed object, eventually by
> removing a point.

It is a toggle. It will be cut at its start point. Notice the number of points info in the status bar.

(In reply to Regina Henschel from comment #13)
> (In reply to Jan-Marek Glogowski from comment #11)
> 
> > There is also the functionality "Close object" in the context menu.
> > Interestingly I couldn't find a way to "Open" a closed object, eventually by
> > removing a point.
> 
> It is a toggle. It will be cut at its start point. Notice the number of
> points info in the status bar.

Where do you see this toggle?

If I select "Close object", the polygon is closed and the entry is removed from the context menu, so no toggle there. I can't find any checkbox / toggle to open it again. I now even tried to search in help, but I guess I use the wrong term - no hit for me.

Created attachment 159274 [details]
Close and open polygon from the context menu

(In reply to Jan-Marek Glogowski from comment #14)
> Where do you see this toggle?

See attached file.

Screenshots from Version: 7.0.0.0.alpha0+ (x64)
Build ID: 6388c578c672690fff662cb04b6a0436cd742f37
CPU threads: 8; OS: Windows 10.0 Build 18362; UI render: default; VCL: win; 
Locale: de-DE (en_US); UI-Language: en-US
Calc: threaded

(In reply to Regina Henschel from comment #15)
> Created attachment 159274 [details]
> Close and open polygon from the context menu
> 
> (In reply to Jan-Marek Glogowski from comment #14)
> > Where do you see this toggle?
> 
> See attached file.
> 
> Screenshots from Version: 7.0.0.0.alpha0+ (x64)
> Build ID: 6388c578c672690fff662cb04b6a0436cd742f37
> CPU threads: 8; OS: Windows 10.0 Build 18362; UI render: default; VCL: win; 
> Locale: de-DE (en_US); UI-Language: en-US
> Calc: threaded

So I just checkout out that exact revision from https://bibisect.libreoffice.org/linux-64-6.5.git, and I still don't see the toggle with either gen, gtk3 or kf5 backend. Interestingly it's named "Close Bézier", after you selected it, but even if I draw a curve instead of a polygon, that entry just vanishes. I'll open a bug report.

(In reply to Regina Henschel from comment #12)
[info about ODF / SVG representation]

I'm not sure, the external representation matters for this bug. But yup - a complex path of different parts (line, curve, arc) can not be constructed in Draw AFAIK, but maybe it's just a hidden feature Armin knows about. It can be done in code, as you can see in the AddPolygonToPath in  vcl/qt5/Qt5Graphics_GDI.cxx. Not sure about the arc segment; maybe it's represented as a "fixed" Bézier curve internally.

> Problems in the UI:
> 1) Using Alt-Click to start a next sequence of points always inserts a
> 'closepath' command. But reading a shape without this 'closepath' command is
> possible.

That's why I suggested the polygon / polyline split of the tool.

> 2) Finish creating with a double-click on the first point, closes the shape
> automatically. But <draw:polyline> elements with same start and end point
> are possible and correctly read.

Same as 1.

> 3) The naming in the UI uses 'Polyline' and 'Polygon' too, when it is a
> path-element with only straight lines.

Hadn't noticed that. Still 1.

> 4) The use of Alt-key with the 'Curves and Polygons'-tool is badly
> communitcated.
> 
> When starting with a polyline and using Alt-Double-click, it does only close
> the shape because of behavior 1).
> Using Ctrl-Alt-Click is the same as only using Alt-Click. It closes the
> shape because of behavior 1) and goes into PolyPolygon-mode. The next line
> will belong to the same shape.

Yup - you need the alternative for "Alt + left-click", as this is the common action to move a window in Linux (Unix?) window managers. In that case you don't need to select the title bar of a window, but can directly start moving the window from the current mouse position.

I've opened bug 131830 for the missing context menu entry mentioned by Regina. Won't link it, as it's not related to this bug.

Don't see how the design group can help here. According comment 12 it's a bunch of issues and you discuss several solutions. Sounds as splitting the ticket into several bugs might be helpful.

(In reply to Heiko Tietze from comment #19)
> Don't see how the design group can help here. According comment 12 it's a
> bunch of issues and you discuss several solutions. Sounds as splitting the
> ticket into several bugs might be helpful.

I think no split is needed. A tool is missing in the UI to create PolyPolylines. The current PolyPoly-modus always generates PolyPolygons.

It is a problem for the design group, because suggestions are needed:
* How should a PolyPoly-modus work from point of view of users?
* How should it work without mouse, or is it only usable with mouse?
* What in the workflow of users distinguishes creating a PolyPolygon from creating a PolyPolyline?
* Which steps should be equal for creating PolyPolyline and PolyPolygon?
* Is a separate tool in the UI needed, or can the current PolyPoly-modus be changed to allow users to choose between creating PolyPolylines and PolyPolygons?
* Should a PolyPoly-modus for other purpose than creating shapes (e.g. Writer contour wrap) have the same workflow?

(In reply to Regina Henschel from comment #20)
> * What in the workflow of users distinguishes creating a PolyPolygon from
> creating a PolyPolyline?

What I can see in the current version (guess it hasnt changed), is a polygon and a filled polygon. For the latter you have to remove the filling to get the wanted closed polygon. Am I wrong?

(In reply to Heiko Tietze from comment #21)
> What I can see in the current version (guess it hasnt changed), is a polygon
> and a filled polygon. For the latter you have to remove the filling to get
> the wanted closed polygon. Am I wrong?

I see currently this behavior: You have the tool uno:Polygon which draws a polygon (which means 'closed'). You have the tool uno:Polygon_unfilled which - despite its name - draws a polyline  (which means 'open'). You finish drawing with double-click. You start a new part in PolyPoly-mode, when you use Alt-click for the last point of the previous part.

The rules for filling are clear and not a problem: For a polygon it depends on the style, whether the polygon is filled or not. A polyline is never filled even if the style has a fill specified.

I see these things to be specified:
(1) How finishes the user a drawing in non PolyPoly-mode?
(2) How differ finishing methods in case there are several ones?
(3) How can a user close a polyline or open a polygon during drawing, especially in PolyPoly-mode?
(4) How does a user enter PolyPoly-mode and start the next part in PolyPolygon-mode?
(5) Is it really intended, that the previous line is automatically closed when the user starts the next part in PolyPoly-mode? See comment 5 
(6) How does a user create a polygon part in PolyPoly-mode if he has started with uno:Polygon_unfilled; and the other way round, how does he create a polyline part if he has started with uno:Polygon?
(7) How does a user finish a PolyPoly-mode drawing?
(8) What is the intended behavior for the modifier keys, for right-click and for double-click? See comment 3
(9) What drawing steps and default styles are bundled with each of the uno commands?
(10) What alternatives should exist when not using the mouse (accessibility)?

The topic was on the agenda of the design meeting.

In general the behavior is similar to MSO and Inkscape with double-click to finish (also Escape for MSO) and modifier keys to change the procedure (somewhat unclear whether ctrl or alt; no obvious way to create polypoly shapes in MSO). 

If we are going to change the behavior it needs to be a convincing improvement. Something that might have an advantage is to add every step (resp. point) to the undo stack so missing the double-click is easy to undo. The drawback is clearly that users who want to undo the whole polygon creation have to click through all steps and may dislike the change.

The recommendation is to keep the workflow as it is and have it well documented. Unless a user joins the discussion (whether here or on ask.libreoffice) all effort is quite academic.

Something that might support the work with polygons is an editable list of attributes. It's also the only way to make this accessible. But IIRC we have tickets on this...