View Issue Details

IDProjectCategoryView StatusLast Update
0034897LazarusTAChartpublic2019-04-17 23:52
ReporterMarcin Wiazowski Assigned Towp  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionno change required 
Product Version2.1 (SVN) 
Summary0034897: TAChart: FClipRect is not always coherent with FCurrentExtent
DescriptionProblem described here is related to 0034819, but has been described separately here, to avoid mess in the original report.


In TChart.PrepareAxis, there is a loop:



FCurrentExtent HAS SOME INTIAL VALUE HERE

      CalculateTransformationCoeffs(ZeroRect);
      cr := FClipRect;
      for tries := 1 to 10 do begin

FClipRect IS ADJUSTED ACCORDING TO THE CURRENT FCurrentExtent VALUE:

        axisMargin := AxisList.Measure(CurrentExtent, scaled_depth);
        axisMargin[calLeft] := Max(axisMargin[calLeft], scaled_depth);
        axisMargin[calBottom] := Max(axisMargin[calBottom], scaled_depth);
        FClipRect := cr;
        for aa := Low(aa) to High(aa) do
          SideByAlignment(FClipRect, aa, -axisMargin[aa]);

        prevExt := FCurrentExtent;
        FCurrentExtent := FLogicalExtent;
        CalculateTransformationCoeffs(GetMargins(ADrawer));

IF FCurrentExtent HAS NOT BEEN CHANGED, WE LEAVE THE LOOP HERE,
AND FClipRect IS STILL COHERENT WITH FCurrentExtent:

        if prevExt = FCurrentExtent then break;

        prevExt := FCurrentExtent;
      end;



The problem is for the last, 10th iteration of the loop - even when FCurrentExtent has been changed, a call to the SideByAlignment is not executed - so FClipRect is no longer coherent with FCurrentExtent.

The attached patch solves the problem.

Regards
TagsNo tags attached.
Fixed in Revision
LazTarget-
WidgetsetWin32/Win64
Attached Files

Activities

Marcin Wiazowski

2019-01-18 21:53

reporter  

patch.diff (1,265 bytes)   
Index: components/tachart/tagraph.pas
===================================================================
--- components/tachart/tagraph.pas	(revision 60103)
+++ components/tachart/tagraph.pas	(working copy)
@@ -1454,17 +1454,25 @@
   // We recalculate them iteratively hoping that the process converges.
   CalculateTransformationCoeffs(ZeroRect);
   cr := FClipRect;
-  for tries := 1 to 10 do begin
-    axisMargin := AxisList.Measure(CurrentExtent, scaled_depth);
+  for tries := 0 to High(tries) do begin
+    axisMargin := AxisList.Measure(FCurrentExtent, scaled_depth);
     axisMargin[calLeft] := Max(axisMargin[calLeft], scaled_depth);
     axisMargin[calBottom] := Max(axisMargin[calBottom], scaled_depth);
     FClipRect := cr;
     for aa := Low(aa) to High(aa) do
       SideByAlignment(FClipRect, aa, -axisMargin[aa]);
+
+    if tries = 10 then
+      Break;
+
     prevExt := FCurrentExtent;
     FCurrentExtent := FLogicalExtent;
     CalculateTransformationCoeffs(GetMargins(ADrawer));
     if prevExt = FCurrentExtent then break;
+
+    // FCurrentExtent has been changed, so FClipRect must be adjusted accordingly -
+    // so we must call SideByAlignment before we are allowed to leave this loop
+
     prevExt := FCurrentExtent;
   end;
 
patch.diff (1,265 bytes)   

wp

2019-01-18 23:36

developer   ~0113466

I know it's difficult, but I'd like to ask you to post a demo which shows that this is a bug and that your patch fixes it.

Marcin Wiazowski

2019-01-18 23:45

reporter   ~0113468

Ok, I'll try to prepare something.

Marcin Wiazowski

2019-01-19 21:59

reporter   ~0113496

Please don't apply the patch.

I made some tests and now I started to suppose, that there is no problem with not updating FClipRect in the last loop iteration - but, on the contrary, the problem is with UPDATING it in every iteration.

Since there exist more urgent problems before releasing RC4, I'll return to this bug report later.

Regards

wp

2019-04-16 17:22

developer   ~0115556

Is this issue still relevant? I am thinking of the recent changes in the graph-image transformation.

Marcin Wiazowski

2019-04-16 18:17

reporter   ~0115559

Yes, this is still relevant.

However, I made some research, that shows, that - due some subtle reasons - its better to not change the current code, unless also some other changes are made in the clip rect calculation process.

In other words, there is a potential to make the chart working better, but this requires some more comprehensive changes.

So I think, that this report can be resolved as "no change required". I want to take a look at this problem again, in a more detailed way, but this requires much time - so rather later than sooner.

wp

2019-04-16 22:14

developer   ~0115566

As requested by reporter

Issue History

Date Modified Username Field Change
2019-01-18 21:53 Marcin Wiazowski New Issue
2019-01-18 21:53 Marcin Wiazowski File Added: patch.diff
2019-01-18 23:33 wp Assigned To => wp
2019-01-18 23:33 wp Status new => assigned
2019-01-18 23:36 wp Note Added: 0113466
2019-01-18 23:36 wp LazTarget => -
2019-01-18 23:36 wp Status assigned => feedback
2019-01-18 23:45 Marcin Wiazowski Note Added: 0113468
2019-01-18 23:45 Marcin Wiazowski Status feedback => assigned
2019-01-19 21:59 Marcin Wiazowski Note Added: 0113496
2019-04-16 17:22 wp Note Added: 0115556
2019-04-16 17:22 wp Status assigned => feedback
2019-04-16 18:17 Marcin Wiazowski Note Added: 0115559
2019-04-16 18:17 Marcin Wiazowski Status feedback => assigned
2019-04-16 22:14 wp Note Added: 0115566
2019-04-16 22:14 wp Status assigned => resolved
2019-04-16 22:14 wp Resolution open => no change required
2019-04-17 23:52 Marcin Wiazowski Status resolved => closed