View Issue Details

IDProjectCategoryView StatusLast Update
0029277LazarusWidgetsetpublic2020-04-01 10:01
ReporterMark Malakanov Assigned ToZeljan Rikalo  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Platformamd64OSlinux 
Product Version1.4.4 
Summary0029277: Suboptimal performance in TQtWidgetSet.DCSetPixel for bitmap based cancases
DescriptionTQtWidgetSet.DCSetPixel uses QPainter_drawPoint for all canvases, even for those that are bitmap based (have vImage, Fot example, in FPC/Lazarus it can be TBitmap.Canvas).
Performance is not perfect because of it.
Also, it uses current Painter's Pen, just changes its color, but does not change other properties like Width etc.
Steps To ReproduceCreate bitmap from TBitmap.
call bitmap.pixels[,y]:=aColor.
Additional InformationI propose to use QT setPixel for bitmap backed canvases. It works at least 4 times faster, and does not depend on Painters Pen, neither changes it.
Also, for screen base d canvases (that in QT have only Painter), it will use specific pen for SetPixel, independent current Painter's pen, and not changing it.current.

file qtobject.inc

class TQtWidgetSet...
...
  FPenForSetPixel: QPenH;
...
procedure TQtWidgetSet.DCSetPixel(CanvasHandle: HDC; X, Y: integer; AColor: TGraphicsColor);
var
  ColorRef: TColorRef;
  Painter: QPainterH;
  Pen:QPenH;
  Rgb:QRgb;
  Color: QColorH;
begin
  if IsValidDC(CanvasHandle) then
  begin
    // WriteLn('TQtWidgetSet.DCSetPixel X=',X,' Y=',Y, ' AColor=',dbghex(AColor),' rgb ? ',dbgHex(ColorToRGB(AColor)));

    if (TQtDeviceContext(CanvasHandle).vImage <> nil) then
      begin
        ColorRef := TColorRef(ColorToRGB(AColor));
        try
          Color:=QColor_create(Red(ColorRef), Green(ColorRef), Blue(ColorRef));
          Rgb:=QColor_rgba(Color);
          QImage_setPixel(TQtDeviceContext(CanvasHandle).vImage.Handle, X,Y, Rgb);
        finally
          QColor_destroy(Color);
        end;
      end
    else if (TQtDeviceContext(CanvasHandle).Widget <> nil) then
      // It is not bitmap. Fallback to Painter drawPoint.
      begin
        Painter := TQtDeviceContext(CanvasHandle).Widget;
        {Save current pen.Better save copy of pen instead of
         using painter save/restore, or saved Pen in devicecontext which
         may be null. Issue 0027620}
        Pen := QPen_create(QPainter_pen(Painter)); //save current pen

        {Create pen for SetPixel from scratch to prevent inheriting Width
         and other Pen properties from Painter current Pen.}
        if FPenForSetPixel = nil then
          FPenForSetPixel := QPen_create;

        try
          ColorRef := TColorRef(ColorToRGB(AColor));
          QColor_fromRgb(@Color, Red(ColorRef), Green(ColorRef), Blue(ColorRef));
          QPen_setColor(FPenForSetPixel, @Color);
          QPainter_setPen(Painter, FPenForSetPixel);
          QPainter_drawPoint(Painter, X,Y);
        finally
          QPainter_setPen(Painter, Pen); //restore saved pen
          QPen_destroy(Pen); //destroy the saved pen.
        end;
      end
     else
      begin
        raise Exception.CreateFmt('TQtDeviceContext(CanvasHandle) %u : does not have vImage or Widget',[PtrUInt(CanvasHandle)]);
      end;
  end;
end;


TagsNo tags attached.
Fixed in Revision62839
LazTarget-
WidgetsetQT, QT5
Attached Files

Activities

Mark Malakanov

2015-12-29 05:42

reporter  

qtobject.inc (61,161 bytes)

Mark Malakanov

2015-12-29 05:44

reporter   ~0088380

Widgetset is QT.

Mark Malakanov

2015-12-29 06:04

reporter  

qt_setPixel_fast.zip (3,191 bytes)

Zeljan Rikalo

2015-12-29 14:11

developer   ~0088396

Please create svn diff patch. http://wiki.freepascal.org/Creating_A_Patch

Mark Malakanov

2015-12-31 01:05

reporter  

patch-0029277.diff (6,967 bytes)   
Index: lcl/interfaces/qt/qtint.pp
===================================================================
--- lcl/interfaces/qt/qtint.pp	(revision 51082)
+++ lcl/interfaces/qt/qtint.pp	(working copy)
@@ -87,6 +87,8 @@
     FWindowManagerName: String; // Track various incompatibilities between WM. Initialized at WS start.
     {$ENDIF}
 
+    FInGetPixel: boolean; //prevent possible recursive call in GetPixel
+    FPenForSetPixel: QPenH;
 
     // qt style does not have pixel metric for themed menubar (menu) height
     // so we must calculate it somehow.
Index: lcl/interfaces/qt/qtobject.inc
===================================================================
--- lcl/interfaces/qt/qtobject.inc	(revision 51082)
+++ lcl/interfaces/qt/qtobject.inc	(working copy)
@@ -1211,18 +1211,54 @@
 
 function TQtWidgetSet.DCGetPixel(CanvasHandle: HDC; X, Y: integer): TGraphicsColor;
 var
-  Color: QColorH;
+  Color: QColorH=nil;
+  Widget: QWidgetH=nil;
+  Pixmap: QPixmapH=nil;
+  Image: QImageH=nil;
+  Painter: QPainterH=nil;
+  Rgb: QRgb=0;
+  WindId: longword=0;
 begin
   Result := clNone;
 
   if not IsValidDC(CanvasHandle) then Exit;
-  
-  if (TQtDeviceContext(CanvasHandle).vImage <> nil) then
-  begin
-    Color := QColor_create(QImage_pixel(TQtDeviceContext(CanvasHandle).vImage.Handle, X, Y));
-    Result := RGBToColor(QColor_red(Color), QColor_green(Color), QColor_blue(Color));
-    QColor_destroy(Color);
-  end;
+
+  if (TQtDeviceContext(CanvasHandle).vImage <> nil)
+    and (TQtDeviceContext(CanvasHandle).vImage.Handle <> nil) then
+    begin
+      Color := QColor_create(QImage_pixel(TQtDeviceContext(CanvasHandle).vImage.Handle, X, Y));
+      Result := RGBToColor(QColor_red(Color), QColor_green(Color), QColor_blue(Color));
+      QColor_destroy(Color);
+    end
+  else if (TQtDeviceContext(CanvasHandle).Widget <> nil) then
+    begin
+      Widget := TQtDeviceContext(CanvasHandle).Parent; //Parent is actually QWidgetH, and Widget is QPainterH.
+      if (Widget = nil) then
+        raise Exception.CreateFmt('TQtDeviceContext(CanvasHandle) %u : does not have Widget',[PtrUInt(CanvasHandle)]);
+      if FInGetPixel then //to prevent recursive call (QPixmap_grabWidget -> Widget.paint -> DCGetPixel)*
+        raise Exception.CreateFmt('TQtWidgetSet.DCGetPixel called recursively, probably from Paint method of Widget %u.',[PtrUInt(Widget)]);
+      FInGetPixel := true;
+      try
+        Pixmap:= QPixmap_create(1,1);
+        Image := QImage_create(1,1, QImageFormat_ARGB32);
+        //QPixmap_grabWidget(Pixmap, Widget, X,Y, 1,1); //this calls Widget.paint directed to bitmap and gets gets pixels from it. Slower.
+        WindId:=QWidget_winId(Widget);
+        QPixmap_grabWindow(Pixmap, WindId ,X,Y,1,1); //this gets pixels from screen.
+        QPixmap_toImage(PixMap,Image);
+        Rgb := QImage_Pixel(Image, 0,0);
+        Color := QColor_create(Rgb);
+        Result := RGBToColor(QColor_red(Color), QColor_green(Color), QColor_blue(Color));
+      finally
+        if Pixmap <> nil then QPixmap_destroy(Pixmap); //It looks like Qt caches pixmap despite of destroy
+        if Image <> nil then QImage_destroy(Image);
+        if Color <> nil then QColor_destroy(Color);
+        FInGetPixel := false;
+      end;
+    end
+    else
+     begin
+       raise Exception.CreateFmt('TQtDeviceContext(CanvasHandle) %u : does not have vImage or Painter',[PtrUInt(CanvasHandle)]);
+     end;
 end;
 
 procedure dbgcolor(msg: string; C:TQColor);
@@ -1232,44 +1268,57 @@
 
 procedure TQtWidgetSet.DCSetPixel(CanvasHandle: HDC; X, Y: integer; AColor: TGraphicsColor);
 var
-  Color: TQColor;
-  AQColor: QColorH;
   ColorRef: TColorRef;
-  Pen: QPenH;
   Painter: QPainterH;
-  ADevType: Integer;
+  Pen:QPenH;
+  Rgb:QRgb;
+  Color: QColorH;
 begin
   if IsValidDC(CanvasHandle) then
   begin
     // WriteLn('TQtWidgetSet.DCSetPixel X=',X,' Y=',Y, ' AColor=',dbghex(AColor),' rgb ? ',dbgHex(ColorToRGB(AColor)));
-    Painter := TQtDeviceContext(CanvasHandle).Widget;
-    ADevType := QPaintDevice_devType(QPaintEngine_paintDevice(QPainter_paintEngine(Painter)));
-    {qt private PaintDeviceFlags 2 = QPixmap 3 = QImage. issue #29256}
-    if ((ADevType = 2) or (ADevType = 3)) and
-     (TQtDeviceContext(CanvasHandle).vImage <> nil) and
-      (TQtDeviceContext(CanvasHandle).vImage.Handle <> nil) then
-    begin
-      ColorRef := TColorRef(ColorToRGB(AColor));
-      QColor_fromRgb(@Color, Red(ColorRef), Green(ColorRef), Blue(ColorRef));
-      AQColor := QColor_create(PQColor(@Color));
-      QImage_setPixel(TQtDeviceContext(CanvasHandle).vImage.Handle, X, Y, QColor_rgb(AQColor));
-      QColor_destroy(AQColor);
-    end else
-    begin
-      {Save current pen.Better save copy of pen instead of
-       using painter save/restore, or saved Pen in devicecontext which
-       may be null. Issue #27620}
-      Pen := QPen_create(QPainter_pen(Painter));
-      try
+
+    if (TQtDeviceContext(CanvasHandle).vImage <> nil)
+      and (TQtDeviceContext(CanvasHandle).vImage.Handle <> nil) then
+      begin
         ColorRef := TColorRef(ColorToRGB(AColor));
-        QColor_fromRgb(@Color, Red(ColorRef), Green(ColorRef), Blue(ColorRef));
-        QPainter_setPen(Painter, PQColor(@Color));
-        QPainter_drawPoint(Painter, X,Y);
-      finally
-        QPainter_setPen(Painter, Pen);
-        QPen_destroy(Pen);
+        Color:=QColor_create(Red(ColorRef), Green(ColorRef), Blue(ColorRef));
+        try
+          Rgb:=QColor_rgba(Color);
+          QImage_setPixel(TQtDeviceContext(CanvasHandle).vImage.Handle, X,Y, Rgb);
+        finally
+          QColor_destroy(Color);
+        end;
+      end
+    else if (TQtDeviceContext(CanvasHandle).Widget <> nil) then
+      // It is not bitmap. Fallback to Painter drawPoint.
+      begin
+        Painter := TQtDeviceContext(CanvasHandle).Widget;
+        {Save current pen.Better save copy of pen instead of
+         using painter save/restore, or saved Pen in devicecontext which
+         may be null. Issue #27620}
+        Pen := QPen_create(QPainter_pen(Painter)); //save current pen
+
+        {Create pen for SetPixel from scratch to prevent inheriting Width
+         and other Pen properties from Painter current Pen.}
+        if FPenForSetPixel = nil then
+          FPenForSetPixel := QPen_create;
+
+        try
+          ColorRef := TColorRef(ColorToRGB(AColor));
+          QColor_fromRgb(@Color, Red(ColorRef), Green(ColorRef), Blue(ColorRef));
+          QPen_setColor(FPenForSetPixel, @Color);
+          QPainter_setPen(Painter, FPenForSetPixel);
+          QPainter_drawPoint(Painter, X,Y);
+        finally
+          QPainter_setPen(Painter, Pen); //restore saved pen
+          QPen_destroy(Pen);  //destroy the saved pen.
+        end;
+      end
+     else
+      begin
+        raise Exception.CreateFmt('TQtDeviceContext(CanvasHandle) %u : does not have vImage or Widget',[PtrUInt(CanvasHandle)]);
       end;
-    end;
   end;
 end;
 
patch-0029277.diff (6,967 bytes)   

Mark Malakanov

2015-12-31 01:06

reporter   ~0088449

Patch is added. But it also includes changes for fix of 0029276.

Zeljan Rikalo

2020-04-01 10:01

developer   ~0121818

Please test and close if ok. Note that Qt5 is also optimized.

Issue History

Date Modified Username Field Change
2015-12-29 05:42 Mark Malakanov New Issue
2015-12-29 05:42 Mark Malakanov File Added: qtobject.inc
2015-12-29 05:44 Mark Malakanov Note Added: 0088380
2015-12-29 06:04 Mark Malakanov File Added: qt_setPixel_fast.zip
2015-12-29 14:10 Zeljan Rikalo Assigned To => Zeljan Rikalo
2015-12-29 14:10 Zeljan Rikalo Status new => assigned
2015-12-29 14:11 Zeljan Rikalo LazTarget => -
2015-12-29 14:11 Zeljan Rikalo Note Added: 0088396
2015-12-29 14:11 Zeljan Rikalo Status assigned => feedback
2015-12-31 01:05 Mark Malakanov File Added: patch-0029277.diff
2015-12-31 01:06 Mark Malakanov Note Added: 0088449
2015-12-31 01:06 Mark Malakanov Status feedback => assigned
2020-04-01 10:01 Zeljan Rikalo Status assigned => resolved
2020-04-01 10:01 Zeljan Rikalo Resolution open => fixed
2020-04-01 10:01 Zeljan Rikalo Fixed in Revision => 62839
2020-04-01 10:01 Zeljan Rikalo Widgetset => QT, QT5
2020-04-01 10:01 Zeljan Rikalo Note Added: 0121818