TCanvas32.DrawPath doesn't handle overlaps

[lamdalili]

DrawPath draws complex figures containing mixed closed and not closed paths in separate passes, so these vectors are treated as separate figures and their overlaps are visible .

The first figure is when the three paths are not closed the second one is given when the path of circle is closed .

var
  img:TImage32;
  C:TCanvas32;
  Stroke:TStrokeBrush;
begin
   img:=TImage32.Create(Self);
   img.AutoSize:=True;
   img.Parent:=Self;
   Img.Bitmap.SetSize(600,400);
   Img.Bitmap.Clear(clWhite32);
   C:=TCanvas32.Create(Img.Bitmap);
   Stroke:= C.Brushes.Add(TStrokeBrush) as TStrokeBrush;
   Stroke.StrokeWidth:=20;
   Stroke.FillColor:=SetAlpha(clRed32,50);
   C.BeginUpdate;
   with C do
   begin
     MoveTo(101,45);//line
     LineTo(226,171);
     EndPath(False);
    MoveTo(163,192);//Circle
     CurveTo(117,191, 79,154, 80,108);
     CurveTo(80,62, 117,24, 163,25);
     CurveTo(210,25, 247,62, 247,108);
     CurveTo(246,155, 209,192, 163,192);
     EndPath(True);
    MoveTo(142,164);// letter P.
     CurveTo(142,164, 142,63, 142,63);
     CurveTo(142,63, 174,63, 174,63);
     CurveTo(188,63, 202,76, 201,91);
     CurveTo(201,106, 188,119, 173,119);
     CurveTo(173,119, 146,119, 146,119);
     EndPath(False);
   end;
   C.EndUpdate;

[andersmelander]

I've thought long and hard about this problem and it seems to me that it's caused by a combination of a design flaw in TCanvas32 and some missing functionality in TStrokeBrush.PolyPolygonFS/BuildPolyPolyLine. As far as I can see, the way those are currently implemented, there's no simple way to solve the problem

One possible solution could be to move the handling of mixed open/closed polylines/polygons from TCanvas32 to TStrokeBrush.PolyPolygonFS or BuildPolyPolyLine. This way all the resulting polylines/polygons could be rendered in one go instead of in multiple passes as they are now. I'll see if I can make that work.

[andersmelander]

I've committed a solution to the bugfix/TCanvas32_DrawPath branch. I used your example as a test case.
I'm thinking this might also have been relevant for #107.

Anyway, give it a try.

[lamdalili]

Thanks for help and code

I simplified the solution by removing PathClosed from TFlattenedPath, which was bad design because there are many other geometric entities need to be treated as a whole with new data structure . So the trick adopted is when the path is closed, the first and the last point must have the same value otherwise make sure they have different values.. this is not the best solution but allows to StrokBrush to work properly without adding more complexity to this limited implementation.

portion modified in TFlattenedPath.EndPath

  if (Close) then
  begin
    AddPoint(FPoints[0]);
    CurrentPoint := FPoints[0];
  end else if(FPoints[0]= FPoints[FPointIndex-1]) then
     FPoints[0].X:=FPoints[0].X+0.0001;

New virual method added TStrokeBrush.BuildStrokePath
Method deleted TDashedBrush.PolyPolygonFS

procedure TStrokeBrush.PolyPolygonFS(Renderer: TCustomPolygonRenderer;
  const Points: TArrayOfArrayOfFloatPoint; const ClipRect: TFloatRect;
  Transformation: TTransformation );
var
  I,L,J,S: Integer;
  Tmp,Polys:TArrayOfArrayOfFloatPoint;
begin
  Polys:=nil;
  S:=0;
  for I := 0 to High(Points) do
  begin
      Tmp:= BuildStrokePath(Points[I]);
      L:=Length(Tmp);
      SetLength(Polys,S+L);
      for J := 0 to L-1 do  //merge
        Polys[S+J]:=Tmp[J];
      inc(S,L);
  end;
  inherited PolyPolygonFS(Renderer, Polys, ClipRect, Transformation);
end;

function TStrokeBrush.BuildStrokePath(const Points: TArrayOfFloatPoint): TArrayOfArrayOfFloatPoint;
var
  t:TArrayOfArrayOfFloatPoint;
begin
  SetLength(t,1);
  t[0]:=Points;
  Result := BuildPolyPolyline(t, IsClosed(Points), StrokeWidth, JoinStyle,
               EndStyle, MiterLimit);
end;

function TDashedBrush.BuildStrokePath(const Points: TArrayOfFloatPoint): TArrayOfArrayOfFloatPoint;
begin
  if Length(FDashArray)<>0 then
  begin
    Result:= BuildDashedLine(Points, FDashArray, FDashOffset,IsClosed(Points));
    Result:= BuildPolyPolyline(Result,false, StrokeWidth, JoinStyle,
              EndStyle, MiterLimit);
  end else
    Result:= inherited;
end;

simple helper IsClosed

function IsClosed(const t: TArrayOfFloatPoint):boolean;
var
Len:integer;
begin
   Len:=Length(t);
   if Len<3 then
     Exit(False);
   Result:=(t[0] = t[Len-1]);
end;

[andersmelander]

I don't think I understand.

Can you please post two code snippets which demonstrate the problem:

1. Draw a figure using TCanvas32 (should exhibit the problem)
2. Draw a figure not using TCanvas32 (should not exhibit the problem)

[lamdalili]

Same code posted in the first post with some tests with TDashedBrush as it has the same problem - this issue is fixed now..

The figure above was my intention to implement ArcLineJoin in the past I abandoned due to the design of TCanvas32 and TStrokeBrush.

[andersmelander]

Same code posted in the first post with some tests with TDashedBrush as it has the same problem - this issue is fixed now..

As far as I can tell this issue is resolved as your problems with arc line joins are beyond the issue scope.

I'll create a new issue about arc line joins.

[lamdalili]

Please delete the last commit even if it solves the problem but it shouldn't be part of the library, the code adds unnecessary complexity for marginal benefit and propagates the design error :)

[andersmelander]

I disagree.
This issue demonstrated a real deficiency in an existing feature, namely the handling of mixed open/closed paths. That issue has now been resolved.

It may very well be that the solution complicates things for your effort to implement arc line joins and that the whole thing will have to be redesigned but we'll cross that bridge when and if we come to it.
So far I have only seen a very vague outline of some TCanvas32 changes and no explanation of how these would enable you to implement arc line joins. This is not enough information to base a decision on.

[lamdalili]

I'm thinking this might also have been relevant for #107.

By default path generated by TextToPath is rendered with oddeven winding rule, so any intersection area becomes visible, mainly in case of cursive font or strikethrough text.

[andersmelander]

By default path generated by TextToPath is rendered with oddeven winding rule

Where?

FWIW, I believe the normal way to render a text poly-polygon would be to use a TSolidBrush with FillMode set to pfNonZero (the default). What am I missing?

[lamdalili]

I believe the normal way to render a text poly-polygon would be to use a TSolidBrush with FillMode set to pfNonZero (the default). What am I missing?

No, it's not possible with the text path at least in the same run, the curent structure used to hold path text is very poor just ArrayOfArrayOfPoint which doesn't help to distinguish it

[andersmelander]

No, it's not possible with the text path at least in the same run

Not possible? Works fine for me:

TStrokeBrush

TSolidBrush

[lamdalili]

I'm not sure understand the last case I haven’t done any test later ,
but the issue 107 is due to that fact.
I'll try understand

thanks

[lamdalili]

Problem not fixed for TDashedBrush currently it produces the same anomaly,, there are three almost identical methods that make it hard to guess which one to implement :

procedure DoPolyPolygonFS(Renderer: TCustomPolygonRenderer;
    const Points: TArrayOfArrayOfFloatPoint;
    const ClipRect: TFloatRect; Transformation: TTransformation;
    Closed: Boolean); virtual;

procedure PolyPolygonFS(Renderer: TCustomPolygonRenderer;
    const Points: TArrayOfArrayOfFloatPoint;
    const ClipRect: TFloatRect; Transformation: TTransformation;
    Closed: Boolean); overload; virtual;

procedure PolyPolygonFS(Renderer: TCustomPolygonRenderer;
    const Points: TArrayOfArrayOfFloatPoint;
    const ClipRect: TFloatRect; Transformation: TTransformation;
    Closed: TBooleanArray); overload; virtual;

I suggest creating a single method having both overloaded paramaters Closed to avoid using global variables FBuffer & FCurrentIndex , so that all the work will be focused around this new method:

in this version if Closeds is present DefaultClosed has no effet

procedure PolyPolygonFS(Renderer: TCustomPolygonRenderer;
      const Points: TArrayOfArrayOfFloatPoint;
      const ClipRect: TFloatRect; Transformation: TTransformation;
      DefaultClosed: Boolean; Closeds: TBooleanArray); overload; virtual;

[andersmelander]

Problem not fixed for TDashedBrush it produce the same anomaly,, there are three almost identical methods that make it hard to guess which one to implement :

Is this the "anomaly" you mean?

I think that case is well beyond what TCanvas32 is meant to handle.

I suspect that what you wanted was for all the dashes to be merged into non-overlapping polygons so they could be drawn in one go without semi-transparency issues, right? If this is what you want then you should merge the polygons before passing them to TCanvas32 (you can use Clipper or something like it for that). Graphics32 itself does not contain functionality to calculate the union of a set of polygons. It does contain a copy of Clipper but Clipper's functionality isn't integrated as that would kill performance.
See the Clipper example.

there are three almost identical methods that make it hard to guess which one to implement

Yes, the TCustomBrush class hierarchy isn't very well designed. My guess is that it was a work-in-progress that was never completed - just like TCanvas32 before I rewrote it. I will create a new issue to consider what can be done about it.

[lamdalili]

I suspect that what you wanted was for all the dashes to be merged into non-overlapping polygons so they could be drawn in one go without semi-transparency issues, right?

Yes it's about that ,
Currently I'm using an old code from old version that I've modified and it works for years,
now I want to update to a new version and see if everything will pass fine, so I noticed the problem and I remembered this old bug.

[andersmelander]

I've had a think about how this can be solved and it has dawned on me that the problem simply is that the polypolygons are rendered too soon.

Right now we render like this (using TStrokeBrush as an example):

• For each brush (TCanvas32.DrawPath):
  • TStrokeBrush
    • For all polypolyline A in the path (TStrokeBrush.PolyPolygonFS):
      • Build a new polypolygon B from the polypolyline A using the stroke settings (BuildPolyPolyLine)
    • Render the polypolygon B (TCustomBrush.PolyPolygonFS)
  • TDashedBrush
    • For each polyline A in the path (TDashedBrush.PolyPolygonFS):
      • Build a new polypolyline B from the polyline A using the dashed stroke settings (BuildDashedLine)
      • Build a new polypolygon C from the dashed polypolyline B using the stroke settings (BuildPolyPolyLine)
      • Render the polypolygon C

The sequence for TStrokeBrush is correct but for TDashedBrush, instead of rendering each polyline after it has been made into a polypolygon, we should be collecting them all into one big polypolygon and then only render that once we have them all. Like this:

• TDashedBrush
  • For each polyline A in the path (TDashedBrush.PolyPolygonFS):
    • Build a new polypolyline B from the polyline A using the dashed stroke settings (BuildDashedLine)
    • Build a new polypolygon C from the dashed polypolyline B using the stroke settings (BuildPolyPolyLine)
    • Add the polypolygon C to the polypolygon D
  • Render the polypolygon D

The change required to implement his should be minimal and will not require any API changes. Let me just try [...2 minutes later...]:

And this is the fix:

procedure TDashedBrush.PolyPolygonFS(Renderer: TCustomPolygonRenderer;
  const Points: TArrayOfArrayOfFloatPoint; const ClipRect: TFloatRect;
  Transformation: TTransformation; Closed: Boolean);
var
  I: Integer;
  DashedLines: TArrayOfArrayOfFloatPoint;
begin
  DashedLines := nil;
  for I := 0 to High(Points) do
    DashedLines := DashedLines + BuildDashedLine(Points[I], FDashArray, FDashOffset, Closed);

  inherited PolyPolygonFS(Renderer, DashedLines, ClipRect, Transformation, False);
end;

There still is a problem with TDashedBrush and mixed open/closed polygons. It seems TDashedBrush incorrectly falls back to TStrokedBrush in this case but that problem is unrelated to the above.

[andersmelander]

There still is a problem with TDashedBrush and mixed open/closed polygons.

Fixed; I have now completely redesigned TCustomBrush. This is a breaking change (if you have implemented custom brushes).

[andersmelander]

if you have implemented custom brushes

I made a few in the Curves example just to dog-food the design.

[lamdalili]

Thanks for your great efforts the second fix works as expected 👍
I tested the first code of TDashBrush branche works in some cases in others it fails completely, it seems that PolyPolygonFS isn't calles at all
here example :

the second design in TCustomBrush-redesign is clearer and easier to guess than the previous implementation of the old fix which was narrowly designed to work with TStrokeBrush, I just have a quick remark about the code of TDashedBrush.ProcessPolyPolygon dashes are always open when passed to the inherited method Closed should be false

graphics32/Source/GR32_Brushes.pas

Lines 667 to 671 in a517ef0

	Result := nil;
	for I := 0 to High(Points) do
	Result := Result + BuildDashedLine(Points[I], FDashArray, FDashOffset, Closed);
	Result := inherited ProcessPolyPolygon(Renderer, Result, ClipRect, Transformation, Closed);

as you can see in the figure

I propose this change :

  if Length(FDashArray) = 0 then
  begin
    Result := inherited ProcessPolyPolygon(Renderer, Points, ClipRect, Transformation, Closed);
  end else
  begin
    Result := nil;
    for I := 0 to High(Points) do
      Result := Result + BuildDashedLine(Points[I], FDashArray, FDashOffset, Closed);

    Result := inherited ProcessPolyPolygon(Renderer, Result, ClipRect, Transformation, False);
  end;

My last remark is the current code is supposed to work with TDashedBrush , TStrokeBrush and TSolidBrushbut doesnt guarantee the support of other type of Brushs , for this it would also be necessary to think about resolving this problem at the level of TRender with multi passes , I think you probably discussed about the subject here #63

the last thing I want to know how to enable Clipper & Angus utilites , I 've juste enable the symbol GR32_OFFSET_CLIPPER in GR32_VectorUtils but this doesn't work ?

when comparing with Firefox broser I get this

Svg file of test
DashStroke.zip

 var
  img:TImage32;
  C:TCanvas32;
  Stroke:TDashedBrush;
  dh: TArrayOfFloat;
begin
   img:=TImage32.Create(Self);
   img.AutoSize:=True;
   img.Parent:=Self;
   Img.Bitmap.SetSize(600,400);
   Img.Bitmap.Clear(clWhite32);
   C:=TCanvas32.Create(Img.Bitmap);
   Stroke:= C.Brushes.Add(TDashedBrush) as TDashedBrush;
   Stroke.StrokeWidth := 20;
   Stroke.SetDashArray([30 ,10]);
   Stroke.FillColor := SetAlpha(clRed32,50);
   C.BeginUpdate;
   with C do
   begin
      MoveTo(129, 198);
      LineTo(432, 198);
      LineTo(432, 131);
      LineTo(157, 134);
      LineTo(129, 198);
      EndPath(false);

      MoveTo(198, 61);
      LineTo(198, 309);
      LineTo(263, 309);
      LineTo(263, 20);
      EndPath(false);

      MoveTo(315, 58);
      LineTo(313, 307);
      LineTo(378, 307);
      LineTo(378, 20);
      EndPath(true);
   end;
   C.EndUpdate;

[andersmelander]

I tested the first code of TDashBrush branche works in some cases in others it fails completely, it seems that PolyPolygonFS isn't calles at all

Yes, that's as-expected with that branch. The TCustomBrush-redesign branch should fix that.

My last remark is the current code is supposed to work with TDashedBrush , TStrokeBrush and TSolidBrushbut doesnt guarantee the support of other type of Brushs , for this it would also be necessary to think about resolving this problem at the level of TRender with multi passes ,

Please explain.

I think you probably discussed about the subject here #63

I have tried various solutions to the fill-and-stroke problem but I have unfortunately not been able to come up with any good solutions.
If you have any ideas, please share them at #63

the last thing I want to know how to enable Clipper & Angus utilites , I 've juste enable the symbol GR32_OFFSET_CLIPPER in GR32_VectorUtils but this doesn't work ?

Enabling GR32_OFFSET_CLIPPER should be enough.
Try changing TCustomBrush.RenderPolyPolygon to also call PolyPolylineFS so you can see what is going on:

procedure TCustomBrush.RenderPolyPolygon(Renderer: TCustomPolygonRenderer; const Points: TArrayOfArrayOfFloatPoint;
  const ClipRect: TFloatRect; Transformation: TTransformation; Closed: Boolean);
begin
  UpdateRenderer(Renderer);
  Renderer.PolyPolygonFS(Points, ClipRect, Transformation);

  PolyPolylineFS(TPolygonRenderer32(Renderer).Bitmap, Points, clBlue32, Closed);
end;

when comparing with Firefox broser I get this

Investigating...

[andersmelander]

GR32_VectorUtils.Reference	GR32_VectorUtils.Clipper2	GR32_VectorUtils.Angus

I don't think this is a problem; It's a corner case with no exact solution. Like the problems in #106. We can keep tweaking the algorithm forever without ever solving all the small quirks.
If you can find a solution that doesn't kill performance you are welcome to give it a go.
Actually, now that I compare the output of GR32_VectorUtils.Reference and GR32_VectorUtils.Angus it looks as if one of the changes suggested in #106 has not been applied.

[andersmelander]

I just applied one of the changes you suggested in #106 and now the output matches that of GR32_VectorUtils.Angus exactly... How about that.

[lamdalili]

Please explain.

I am nor sure but as you can see the three mentioned brushs TDashedBrush , TStrokeBrush and TSolidBrushbut are based on vectors are easy to draw in any order but i guess this is not possible for other situations you can take a look at TNestedBrush there are several consecutive calls to PolyPolygonFS and I can guess not all rendreing operations can be deferred, so it is not entirely true to say that the old design is wrong since it remains possible, in the worste case, that the drawing process uses a temporary buffer and after that alpha component should be corrected.

Enabling GR32_OFFSET_CLIPPER should be enough.

When I enable the derective and debug BuildPolyPolyLine called inTStrokeBrush.ProcessPolyPolygonI am directly at GR32_VectorUtils.Reference and there is no change in the output ...

I don't think this is a problem; It's a corner case with no exact solution.

I am not worried about that I know that the basic Grow doesn't generate perfect offsets, for performance reasons the operation is shortened, I just want to see how the production of Clipper looks like.

Like the problems in #106. We can keep tweaking the algorithm forever without ever solving all the small quirks.

The problem in 106 cannot appear with Clipper it's perfectly handled, if it's present is for other reason as you showed in the last photos, these irregularities look more like a wraped dash rather offsetting bug.

How about that.

not yet tested I will try again

[andersmelander]

I can guess not all rendreing operations can be deferred, so it is not entirely true to say that the old design is wrong since it remains possible, in the worste case, that the drawing process uses a temporary buffer and after that alpha component should be corrected.

Yes. I see your point.

The brush system really is only meant to handle vector operations via TCanvas32. So a nested brush, like for example TGrowBrush, is only meant to contain other vector brushes.
You can misuse the brushes to do pixel operations, like I do in TDotBrush in the Curves examples, but they are not designed for that.

type
  // Draws a single dot at each vertex
  TDotBrush = class(TCustomBrush)
  strict private
    FColor: TColor32;
  private
    procedure SetColor(const Value: TColor32);
  protected
    procedure RenderPolyPolygon(Renderer: TCustomPolygonRenderer; const Points: TArrayOfArrayOfFloatPoint;
      const ClipRect: TFloatRect; Transformation: TTransformation; Closed: Boolean); override;
  public
    constructor Create(BrushCollection: TBrushCollection); override;

    property Color: TColor32 read FColor write SetColor;
  end;

constructor TDotBrush.Create(BrushCollection: TBrushCollection);
begin
  inherited;
  FColor := clWhite32;
end;

procedure TDotBrush.RenderPolyPolygon(Renderer: TCustomPolygonRenderer; const Points: TArrayOfArrayOfFloatPoint;
  const ClipRect: TFloatRect; Transformation: TTransformation; Closed: Boolean);
var
  i, j: integer;
  Bitmap: TCustomBitmap32;
begin
  Bitmap := (Renderer as TPolygonRenderer32).Bitmap;
  for i := 0 to High(Points) do
    for j := 0 to High(Points[i]) do
      Bitmap.PixelFS[Points[i, j].X, Points[i, j].Y] := FColor;
end;

procedure TDotBrush.SetColor(const Value: TColor32);
begin
  if (FColor = Value) then
    exit;
  FColor := Value;
  Changed;
end;

GR32_OFFSET_CLIPPER

You should be able to see in Delphi if the symbol is defined. The editor should gray out the other units:

[lamdalili]

Thanks now it works without problem.

[lamdalili]

GR32_DEBUG_BRUSH was left enabled in the merged code.

and the name RenderPolyPolygon is incorrect in TDotBrush example