View Issue Details

IDProjectCategoryView StatusLast Update
0035029LazarusWidgetsetpublic2019-09-04 05:43
ReporterZoë PetersonAssigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Product Version2.0Product Build 
Target VersionFixed in Version 
Summary0035029: ListView support for multi-resolution image lists
DescriptionOn Cocoa, when a TListView is associated with a TImageList that includes multiple resolutions, it always uses the base imagelist size. The attached patch adds support for including all of the image resolutions in the associated NSImage so the list view will dynamically switch to the appropriate one when switching between HiDPI and standard displays.

Since the existing Cocoa code didn't rely on TWSCustomListView.SetImageList, this doesn't either, and just always includes all of the available resolutions. It has the benefit of working automatically thanks to Cocoa's built-in switching, and keeps the patch size small and limited to LCL/Interfaces/Cocoa. The alternative would be to update TCustomListView.SetImageListWS to take GetCanvasScaleFactor into account, but there isn't currently an LCL level notification when the scaling factor changes (which is available through NSWindowDelegate.windowDidChangeBackingProperties)
Steps To ReproduceThe attached sample project has a list view and a multi-resolution image list. To demonstrate the behavior more the image list's 16x16 and 32x32 images are slightly different. If you run it on a system with a 96dpi display attached, it will use the 16x16 icons. If you run it on a high DPI/retina display, it will use the 32x32 icons. If you have both types of monitors attached to one computer it will switch dynamically when the window is dragged between them. Setting the ListView's SmallImagesWidth works correctly.
Additional InformationTCocoaWSMenuItem would benefit from similar behavior and should probably use multi-resolution NSImages even if ListView becomes backingScaleFactor aware in the future. Since Mac menus don't generally have images, and the current menu item code is completely different, I haven't looked into that further.
TagsNo tags attached.
Fixed in Revision
LazTarget
WidgetsetCocoa
Attached Files
  • multireslistview.patch (3,826 bytes)
    Index: cocoawscomctrls.pas
    ===================================================================
    --- cocoawscomctrls.pas	(revision 23678)
    +++ cocoawscomctrls.pas	(working copy)
    @@ -1511,48 +1511,56 @@
     var
       bmp : TBitmap;
       lst : TCustomImageList;
    +  lstres: TCustomImageListResolution;
    +  w   : Integer;
    +  sz  : NSSize;
       x,y : integer;
       img : NSImage;
       rep : NSBitmapImageRep;
       cb  : TCocoaBitmap;
     begin
    +  img := nil;
       lst := TSmallImagesAccess(listView).SmallImages;
    +  w := lst.WidthForPPI[TSmallImagesAccess(listView).SmallImagesWidth, 96];
    +  sz.width := w;
    +  sz.height := lst.HeightForWidth[w];
       bmp := TBitmap.Create;
       try
    -    lst.GetBitmap(imgIdx, bmp);
    +    for lstres in lst.Resolutions do begin
    +      lstres.GetBitmap(imgIdx, bmp);
     
    -    if bmp.Handle = 0 then begin
    -      Result := nil;
    -      Exit;
    -    end;
    +      if bmp.Handle = 0 then
    +        Continue;
     
    -    // Bitmap Handle should be nothing but TCocoaBitmap
    -    cb := TCocoaBitmap(bmp.Handle);
    +      // Bitmap Handle should be nothing but TCocoaBitmap
    +      cb := TCocoaBitmap(bmp.Handle);
     
    -    // There's NSBitmapImageRep in TCocoaBitmap, but it depends on the availability
    -    // of memory buffer stored with TCocoaBitmap. As soon as TCocoaBitmap is freed
    -    // pixels are not available. For this reason, we're making a copy of the bitmapdata
    -    // allowing Cocoa to allocate its own buffer (by passing nil for planes parameter)
    -    rep := NSBitmapImageRep(NSBitmapImageRep.alloc).initWithBitmapDataPlanes_pixelsWide_pixelsHigh__colorSpaceName_bitmapFormat_bytesPerRow_bitsPerPixel(
    -      nil, // planes, BitmapDataPlanes
    -      Round(cb.ImageRep.size.Width), // width, pixelsWide
    -      Round(cb.ImageRep.size.Height),// height, PixelsHigh
    -      cb.ImageRep.bitsPerSample,// bitsPerSample, bps
    -      cb.ImageRep.samplesPerPixel, // samplesPerPixel, spp
    -      cb.ImageRep.hasAlpha, // hasAlpha
    -      False, // isPlanar
    -      cb.ImageRep.colorSpaceName, // colorSpaceName
    -      cb.ImageRep.bitmapFormat, // bitmapFormat
    -      cb.ImageRep.bytesPerRow, // bytesPerRow
    -      cb.ImageRep.BitsPerPixel //bitsPerPixel
    -    );
    -    System.Move( cb.ImageRep.bitmapData^, rep.bitmapData^, cb.ImageRep.bytesPerRow * Round(cb.ImageRep.size.height));
    -    img := NSImage(NSImage.alloc).initWithSize( rep.size );
    -    img.addRepresentation(rep);
    -    Result := img;
    +      // There's NSBitmapImageRep in TCocoaBitmap, but it depends on the availability
    +      // of memory buffer stored with TCocoaBitmap. As soon as TCocoaBitmap is freed
    +      // pixels are not available. For this reason, we're making a copy of the bitmapdata
    +      // allowing Cocoa to allocate its own buffer (by passing nil for planes parameter)
    +      rep := NSBitmapImageRep(NSBitmapImageRep.alloc).initWithBitmapDataPlanes_pixelsWide_pixelsHigh__colorSpaceName_bitmapFormat_bytesPerRow_bitsPerPixel(
    +        nil, // planes, BitmapDataPlanes
    +        Round(cb.ImageRep.size.Width), // width, pixelsWide
    +        Round(cb.ImageRep.size.Height),// height, PixelsHigh
    +        cb.ImageRep.bitsPerSample,// bitsPerSample, bps
    +        cb.ImageRep.samplesPerPixel, // samplesPerPixel, spp
    +        cb.ImageRep.hasAlpha, // hasAlpha
    +        False, // isPlanar
    +        cb.ImageRep.colorSpaceName, // colorSpaceName
    +        cb.ImageRep.bitmapFormat, // bitmapFormat
    +        cb.ImageRep.bytesPerRow, // bytesPerRow
    +        cb.ImageRep.BitsPerPixel //bitsPerPixel
    +      );
    +      System.Move( cb.ImageRep.bitmapData^, rep.bitmapData^, cb.ImageRep.bytesPerRow * Round(cb.ImageRep.size.height));
    +      if img = nil then
    +        img := NSImage(NSImage.alloc).initWithSize( sz );
    +      img.addRepresentation(rep);
    +    end;
       finally
         bmp.Free;
       end;
    +  Result := img;
     end;
     
     procedure TLCLListViewCallback.SetItemTextAt(ARow, ACol: Integer;
    
    multireslistview.patch (3,826 bytes)
  • MultiResListView-Example.zip (8,410 bytes)
  • multireslistview-2.patch (4,281 bytes)
    --- a/lcl/interfaces/cocoa/cocoawscomctrls.pas
    +++ b/lcl/interfaces/cocoa/cocoawscomctrls.pas
    @@ -269,6 +269,8 @@ type
       published
       end;
     
    +function AllocMultiResImageFromImageList(lst: TCustomImageList; width, imgIdx: Integer): NSImage;
    +
     implementation
     
     type
    @@ -1680,51 +1682,66 @@ type
       TSmallImagesAccess = class(TCustomListView);
     
     function TLCLListViewCallback.GetImageFromIndex(imgIdx: Integer): NSImage;
    +begin
    +  Result := AllocMultiResImageFromImageList(
    +    TSmallImagesAccess(listView).SmallImages,
    +    TSmallImagesAccess(listView).SmallImagesWidth,
    +    imgIdx);
    +end;
    +
    +function AllocMultiResImageFromImageList(lst: TCustomImageList;
    +  width, imgIdx: Integer): NSImage;
     var
       bmp : TBitmap;
    -  lst : TCustomImageList;
    +  lstres: TCustomImageListResolution;
    +  w   : Integer;
    +  sz  : NSSize;
       x,y : integer;
       img : NSImage;
       rep : NSBitmapImageRep;
       cb  : TCocoaBitmap;
     begin
    -  lst := TSmallImagesAccess(listView).SmallImages;
    +  img := nil;
    +  w := lst.WidthForPPI[width, 96];
    +  sz.width := w;
    +  sz.height := lst.HeightForWidth[w];
       bmp := TBitmap.Create;
       try
    -    lst.GetBitmap(imgIdx, bmp);
    -
    -    if bmp.Handle = 0 then begin
    -      Result := nil;
    -      Exit;
    +    for lstres in lst.Resolutions do begin
    +      lstres.GetBitmap(imgIdx, bmp);
    +
    +      if bmp.Handle = 0 then
    +        Continue;
    +
    +      // Bitmap Handle should be nothing but TCocoaBitmap
    +      cb := TCocoaBitmap(bmp.Handle);
    +
    +      // There's NSBitmapImageRep in TCocoaBitmap, but it depends on the availability
    +      // of memory buffer stored with TCocoaBitmap. As soon as TCocoaBitmap is freed
    +      // pixels are not available. For this reason, we're making a copy of the bitmapdata
    +      // allowing Cocoa to allocate its own buffer (by passing nil for planes parameter)
    +      rep := NSBitmapImageRep(NSBitmapImageRep.alloc).initWithBitmapDataPlanes_pixelsWide_pixelsHigh__colorSpaceName_bitmapFormat_bytesPerRow_bitsPerPixel(
    +        nil, // planes, BitmapDataPlanes
    +        Round(cb.ImageRep.size.Width), // width, pixelsWide
    +        Round(cb.ImageRep.size.Height),// height, PixelsHigh
    +        cb.ImageRep.bitsPerSample,// bitsPerSample, bps
    +        cb.ImageRep.samplesPerPixel, // samplesPerPixel, spp
    +        cb.ImageRep.hasAlpha, // hasAlpha
    +        False, // isPlanar
    +        cb.ImageRep.colorSpaceName, // colorSpaceName
    +        cb.ImageRep.bitmapFormat, // bitmapFormat
    +        cb.ImageRep.bytesPerRow, // bytesPerRow
    +        cb.ImageRep.BitsPerPixel //bitsPerPixel
    +      );
    +      System.Move( cb.ImageRep.bitmapData^, rep.bitmapData^, cb.ImageRep.bytesPerRow * Round(cb.ImageRep.size.height));
    +      if img = nil then
    +        img := NSImage(NSImage.alloc).initWithSize( sz );
    +      img.addRepresentation(rep);
         end;
    -
    -    // Bitmap Handle should be nothing but TCocoaBitmap
    -    cb := TCocoaBitmap(bmp.Handle);
    -
    -    // There's NSBitmapImageRep in TCocoaBitmap, but it depends on the availability
    -    // of memory buffer stored with TCocoaBitmap. As soon as TCocoaBitmap is freed
    -    // pixels are not available. For this reason, we're making a copy of the bitmapdata
    -    // allowing Cocoa to allocate its own buffer (by passing nil for planes parameter)
    -    rep := NSBitmapImageRep(NSBitmapImageRep.alloc).initWithBitmapDataPlanes_pixelsWide_pixelsHigh__colorSpaceName_bitmapFormat_bytesPerRow_bitsPerPixel(
    -      nil, // planes, BitmapDataPlanes
    -      Round(cb.ImageRep.size.Width), // width, pixelsWide
    -      Round(cb.ImageRep.size.Height),// height, PixelsHigh
    -      cb.ImageRep.bitsPerSample,// bitsPerSample, bps
    -      cb.ImageRep.samplesPerPixel, // samplesPerPixel, spp
    -      cb.ImageRep.hasAlpha, // hasAlpha
    -      False, // isPlanar
    -      cb.ImageRep.colorSpaceName, // colorSpaceName
    -      cb.ImageRep.bitmapFormat, // bitmapFormat
    -      cb.ImageRep.bytesPerRow, // bytesPerRow
    -      cb.ImageRep.BitsPerPixel //bitsPerPixel
    -    );
    -    System.Move( cb.ImageRep.bitmapData^, rep.bitmapData^, cb.ImageRep.bytesPerRow * Round(cb.ImageRep.size.height));
    -    img := NSImage(NSImage.alloc).initWithSize( rep.size );
    -    img.addRepresentation(rep);
    -    Result := img;
       finally
         bmp.Free;
       end;
    +  Result := img;
     end;
     
     procedure TLCLListViewCallback.SetItemTextAt(ARow, ACol: Integer;
    
    multireslistview-2.patch (4,281 bytes)

Relationships

related to 0034702 new Cocoa: in Retina video mode (hi dpi), buttons look broken 

Activities

Zoë Peterson

2019-02-06 22:56

reporter  

multireslistview.patch (3,826 bytes)
Index: cocoawscomctrls.pas
===================================================================
--- cocoawscomctrls.pas	(revision 23678)
+++ cocoawscomctrls.pas	(working copy)
@@ -1511,48 +1511,56 @@
 var
   bmp : TBitmap;
   lst : TCustomImageList;
+  lstres: TCustomImageListResolution;
+  w   : Integer;
+  sz  : NSSize;
   x,y : integer;
   img : NSImage;
   rep : NSBitmapImageRep;
   cb  : TCocoaBitmap;
 begin
+  img := nil;
   lst := TSmallImagesAccess(listView).SmallImages;
+  w := lst.WidthForPPI[TSmallImagesAccess(listView).SmallImagesWidth, 96];
+  sz.width := w;
+  sz.height := lst.HeightForWidth[w];
   bmp := TBitmap.Create;
   try
-    lst.GetBitmap(imgIdx, bmp);
+    for lstres in lst.Resolutions do begin
+      lstres.GetBitmap(imgIdx, bmp);
 
-    if bmp.Handle = 0 then begin
-      Result := nil;
-      Exit;
-    end;
+      if bmp.Handle = 0 then
+        Continue;
 
-    // Bitmap Handle should be nothing but TCocoaBitmap
-    cb := TCocoaBitmap(bmp.Handle);
+      // Bitmap Handle should be nothing but TCocoaBitmap
+      cb := TCocoaBitmap(bmp.Handle);
 
-    // There's NSBitmapImageRep in TCocoaBitmap, but it depends on the availability
-    // of memory buffer stored with TCocoaBitmap. As soon as TCocoaBitmap is freed
-    // pixels are not available. For this reason, we're making a copy of the bitmapdata
-    // allowing Cocoa to allocate its own buffer (by passing nil for planes parameter)
-    rep := NSBitmapImageRep(NSBitmapImageRep.alloc).initWithBitmapDataPlanes_pixelsWide_pixelsHigh__colorSpaceName_bitmapFormat_bytesPerRow_bitsPerPixel(
-      nil, // planes, BitmapDataPlanes
-      Round(cb.ImageRep.size.Width), // width, pixelsWide
-      Round(cb.ImageRep.size.Height),// height, PixelsHigh
-      cb.ImageRep.bitsPerSample,// bitsPerSample, bps
-      cb.ImageRep.samplesPerPixel, // samplesPerPixel, spp
-      cb.ImageRep.hasAlpha, // hasAlpha
-      False, // isPlanar
-      cb.ImageRep.colorSpaceName, // colorSpaceName
-      cb.ImageRep.bitmapFormat, // bitmapFormat
-      cb.ImageRep.bytesPerRow, // bytesPerRow
-      cb.ImageRep.BitsPerPixel //bitsPerPixel
-    );
-    System.Move( cb.ImageRep.bitmapData^, rep.bitmapData^, cb.ImageRep.bytesPerRow * Round(cb.ImageRep.size.height));
-    img := NSImage(NSImage.alloc).initWithSize( rep.size );
-    img.addRepresentation(rep);
-    Result := img;
+      // There's NSBitmapImageRep in TCocoaBitmap, but it depends on the availability
+      // of memory buffer stored with TCocoaBitmap. As soon as TCocoaBitmap is freed
+      // pixels are not available. For this reason, we're making a copy of the bitmapdata
+      // allowing Cocoa to allocate its own buffer (by passing nil for planes parameter)
+      rep := NSBitmapImageRep(NSBitmapImageRep.alloc).initWithBitmapDataPlanes_pixelsWide_pixelsHigh__colorSpaceName_bitmapFormat_bytesPerRow_bitsPerPixel(
+        nil, // planes, BitmapDataPlanes
+        Round(cb.ImageRep.size.Width), // width, pixelsWide
+        Round(cb.ImageRep.size.Height),// height, PixelsHigh
+        cb.ImageRep.bitsPerSample,// bitsPerSample, bps
+        cb.ImageRep.samplesPerPixel, // samplesPerPixel, spp
+        cb.ImageRep.hasAlpha, // hasAlpha
+        False, // isPlanar
+        cb.ImageRep.colorSpaceName, // colorSpaceName
+        cb.ImageRep.bitmapFormat, // bitmapFormat
+        cb.ImageRep.bytesPerRow, // bytesPerRow
+        cb.ImageRep.BitsPerPixel //bitsPerPixel
+      );
+      System.Move( cb.ImageRep.bitmapData^, rep.bitmapData^, cb.ImageRep.bytesPerRow * Round(cb.ImageRep.size.height));
+      if img = nil then
+        img := NSImage(NSImage.alloc).initWithSize( sz );
+      img.addRepresentation(rep);
+    end;
   finally
     bmp.Free;
   end;
+  Result := img;
 end;
 
 procedure TLCLListViewCallback.SetItemTextAt(ARow, ACol: Integer;
multireslistview.patch (3,826 bytes)

Zoë Peterson

2019-02-06 22:56

reporter  

MultiResListView-Example.zip (8,410 bytes)

Zoë Peterson

2019-09-03 23:58

reporter   ~0117941

I needed an ImageList to multi-resolution NSImage conversion elsewhere in my code, so this version of the patch has the behavior extracted out into a standalone function. I don't think this is the best place for it, but it wasn't obvious which unit would be better for it, and I don't want to make the patch larger than it needs to be if it's not going to be merged soon.

multireslistview-2.patch (4,281 bytes)
--- a/lcl/interfaces/cocoa/cocoawscomctrls.pas
+++ b/lcl/interfaces/cocoa/cocoawscomctrls.pas
@@ -269,6 +269,8 @@ type
   published
   end;
 
+function AllocMultiResImageFromImageList(lst: TCustomImageList; width, imgIdx: Integer): NSImage;
+
 implementation
 
 type
@@ -1680,51 +1682,66 @@ type
   TSmallImagesAccess = class(TCustomListView);
 
 function TLCLListViewCallback.GetImageFromIndex(imgIdx: Integer): NSImage;
+begin
+  Result := AllocMultiResImageFromImageList(
+    TSmallImagesAccess(listView).SmallImages,
+    TSmallImagesAccess(listView).SmallImagesWidth,
+    imgIdx);
+end;
+
+function AllocMultiResImageFromImageList(lst: TCustomImageList;
+  width, imgIdx: Integer): NSImage;
 var
   bmp : TBitmap;
-  lst : TCustomImageList;
+  lstres: TCustomImageListResolution;
+  w   : Integer;
+  sz  : NSSize;
   x,y : integer;
   img : NSImage;
   rep : NSBitmapImageRep;
   cb  : TCocoaBitmap;
 begin
-  lst := TSmallImagesAccess(listView).SmallImages;
+  img := nil;
+  w := lst.WidthForPPI[width, 96];
+  sz.width := w;
+  sz.height := lst.HeightForWidth[w];
   bmp := TBitmap.Create;
   try
-    lst.GetBitmap(imgIdx, bmp);
-
-    if bmp.Handle = 0 then begin
-      Result := nil;
-      Exit;
+    for lstres in lst.Resolutions do begin
+      lstres.GetBitmap(imgIdx, bmp);
+
+      if bmp.Handle = 0 then
+        Continue;
+
+      // Bitmap Handle should be nothing but TCocoaBitmap
+      cb := TCocoaBitmap(bmp.Handle);
+
+      // There's NSBitmapImageRep in TCocoaBitmap, but it depends on the availability
+      // of memory buffer stored with TCocoaBitmap. As soon as TCocoaBitmap is freed
+      // pixels are not available. For this reason, we're making a copy of the bitmapdata
+      // allowing Cocoa to allocate its own buffer (by passing nil for planes parameter)
+      rep := NSBitmapImageRep(NSBitmapImageRep.alloc).initWithBitmapDataPlanes_pixelsWide_pixelsHigh__colorSpaceName_bitmapFormat_bytesPerRow_bitsPerPixel(
+        nil, // planes, BitmapDataPlanes
+        Round(cb.ImageRep.size.Width), // width, pixelsWide
+        Round(cb.ImageRep.size.Height),// height, PixelsHigh
+        cb.ImageRep.bitsPerSample,// bitsPerSample, bps
+        cb.ImageRep.samplesPerPixel, // samplesPerPixel, spp
+        cb.ImageRep.hasAlpha, // hasAlpha
+        False, // isPlanar
+        cb.ImageRep.colorSpaceName, // colorSpaceName
+        cb.ImageRep.bitmapFormat, // bitmapFormat
+        cb.ImageRep.bytesPerRow, // bytesPerRow
+        cb.ImageRep.BitsPerPixel //bitsPerPixel
+      );
+      System.Move( cb.ImageRep.bitmapData^, rep.bitmapData^, cb.ImageRep.bytesPerRow * Round(cb.ImageRep.size.height));
+      if img = nil then
+        img := NSImage(NSImage.alloc).initWithSize( sz );
+      img.addRepresentation(rep);
     end;
-
-    // Bitmap Handle should be nothing but TCocoaBitmap
-    cb := TCocoaBitmap(bmp.Handle);
-
-    // There's NSBitmapImageRep in TCocoaBitmap, but it depends on the availability
-    // of memory buffer stored with TCocoaBitmap. As soon as TCocoaBitmap is freed
-    // pixels are not available. For this reason, we're making a copy of the bitmapdata
-    // allowing Cocoa to allocate its own buffer (by passing nil for planes parameter)
-    rep := NSBitmapImageRep(NSBitmapImageRep.alloc).initWithBitmapDataPlanes_pixelsWide_pixelsHigh__colorSpaceName_bitmapFormat_bytesPerRow_bitsPerPixel(
-      nil, // planes, BitmapDataPlanes
-      Round(cb.ImageRep.size.Width), // width, pixelsWide
-      Round(cb.ImageRep.size.Height),// height, PixelsHigh
-      cb.ImageRep.bitsPerSample,// bitsPerSample, bps
-      cb.ImageRep.samplesPerPixel, // samplesPerPixel, spp
-      cb.ImageRep.hasAlpha, // hasAlpha
-      False, // isPlanar
-      cb.ImageRep.colorSpaceName, // colorSpaceName
-      cb.ImageRep.bitmapFormat, // bitmapFormat
-      cb.ImageRep.bytesPerRow, // bytesPerRow
-      cb.ImageRep.BitsPerPixel //bitsPerPixel
-    );
-    System.Move( cb.ImageRep.bitmapData^, rep.bitmapData^, cb.ImageRep.bytesPerRow * Round(cb.ImageRep.size.height));
-    img := NSImage(NSImage.alloc).initWithSize( rep.size );
-    img.addRepresentation(rep);
-    Result := img;
   finally
     bmp.Free;
   end;
+  Result := img;
 end;
 
 procedure TLCLListViewCallback.SetItemTextAt(ARow, ACol: Integer;
multireslistview-2.patch (4,281 bytes)

jamie philbrook

2019-09-04 02:45

reporter   ~0117942

I find it hard to believe Cocoa does not have a way to trigger a notification?

 In Windows the WM_SETTINGCHANGE message is sent to all top level windows to notify of system setting changes and that includes display properties.

 also that includes WM_DEVICECHANGE and WM_DISPLAYCHANGE

Zoë Peterson

2019-09-04 05:43

reporter   ~0117943

Jamie: Cocoa does, the LCL just doesn't do anything with the notification yet

Issue History

Date Modified Username Field Change
2019-02-06 22:56 Zoë Peterson New Issue
2019-02-06 22:56 Zoë Peterson File Added: multireslistview.patch
2019-02-06 22:56 Zoë Peterson File Added: MultiResListView-Example.zip
2019-04-18 06:03 Dmitry Boyarintsev Relationship added related to 0034702
2019-09-03 23:58 Zoë Peterson File Added: multireslistview-2.patch
2019-09-03 23:58 Zoë Peterson Note Added: 0117941
2019-09-04 02:45 jamie philbrook Note Added: 0117942
2019-09-04 05:43 Zoë Peterson Note Added: 0117943