[PATCH] Autocompletion list image scaling for HiDPI/Retina displays

29 views
Skip to first unread message

Mitchell

unread,
May 10, 2025, 11:46:49 AMMay 10
to scintilla-interest
Hi Neil,

Attached is a patch and zip against Scintilla 5.5.6 that implements SCI_AUTOCSETIMAGESCALE/SCI_AUTOCGETIMAGESCALE for Qt and GTK 3.10+ along with documentation, based on our previous thread[1].

The only questionable bit in my opinion is the GTK implementation switches the list store image column type to CAIRO_GOBJECT_TYPE_SURFACE from GDK_TYPE_PIXBUF. Not only does this require the <cairo/cairo-gobject.h> header (which is included with my default libgtk-3-dev installation), but it also performs a copy for each pixbuf to be included in the list. This copy is necessary because Cairo surfaces store device scale, not GDK Pixbufs[2]. The list store takes ownership of the surface (it’s a boxed pointer), and will free it when done[3]. I was not able to figure out how to create a Cairo surface from an incoming pixbuf, g_object_ref() it, and pass it to the list store directly (thus avoiding copies and a premature deallocation).

For the Qt version, you’ll notice I’m making `im.width() / im.devicePixelRatio()` calls instead of `im.deviceIndependentSize().width()`. That is because the latter was introduced in Qt 6.2, but Scintilla still supports Qt 5.

Thanks for your consideration!

Cheers,
Mitchell

[1]: https://groups.google.com/g/scintilla-interest/c/hxSnbpnjED4
[2]: https://gitlab.gnome.org/GNOME/gtk/-/issues/613
[3]: https://gitlab.gnome.org/GNOME/gtk/-/issues/613#note_687918

auto-c-image-scale.patch
auto-c-image-scale.zip

Mitchell

unread,
May 10, 2025, 11:50:42 AMMay 10
to scintilla...@googlegroups.com

> On May 10, 2025, at 11:46 AM, Mitchell <co...@foicica.com> wrote:
>
> The only questionable bit in my opinion is the GTK implementation switches the list store image column type to CAIRO_GOBJECT_TYPE_SURFACE from GDK_TYPE_PIXBUF. Not only does this require the <cairo/cairo-gobject.h> header (which is included with my default libgtk-3-dev installation), but it also performs a copy for each pixbuf to be included in the list. This copy is necessary because Cairo surfaces store device scale, not GDK Pixbufs[2].

I meant “this column type is necessary” not “this copy is necessary”.

Mitchell

Neil Hodgson

unread,
May 11, 2025, 11:31:37 PMMay 11
to scintilla...@googlegroups.com
Hi Mitchell,

> Attached is a patch and zip against Scintilla 5.5.6 that implements ...

There are some warnings from compilers and linters so I made some more
changes that silenced these.

Type warnings and missing break from compilers:
--- a/src/ScintillaBase.cxx Mon May 12 11:38:13 2025 +1000
+++ b/src/ScintillaBase.cxx Mon May 12 13:20:11 2025 +1000
@@ -974,10 +974,11 @@
return vs.autocStyle;

case Message::AutoCSetImageScale:
- ac.imageScale = static_cast<int>(wParam) / 100.0;
+ ac.imageScale = static_cast<float>(wParam) / 100.0f;
+ break;

case Message::AutoCGetImageScale:
- return ac.imageScale * 100;
+ return static_cast<int>(ac.imageScale * 100);

case Message::RegisterImage:
ac.lb->RegisterImage(static_cast<int>(wParam), ConstCharPtrFromSPtr(lParam));

No initialisation from cppcheck:
--- a/src/Platform.h Mon May 12 11:38:13 2025 +1000
+++ b/src/Platform.h Mon May 12 13:20:11 2025 +1000
@@ -317,7 +317,7 @@
std::optional<ColourRGBA> foreSelected;
std::optional<ColourRGBA> backSelected;
AutoCompleteOption options=AutoCompleteOption::Normal;
- float imageScale;
+ float imageScale=1.0f;
};

class ListBox : public Window {

scripts/HeaderCheck.py:
--- a/scripts/HeaderOrder.txt Mon May 12 11:38:13 2025 +1000
+++ b/scripts/HeaderOrder.txt Mon May 12 13:20:11 2025 +1000
@@ -71,6 +71,7 @@
#include <gdk/gdkkeysyms.h>
#include <gdk/gdkwayland.h>
#include <gtk/gtk-a11y.h>
+#include <cairo/cairo-gobject.h>

// Windows headers
#include <windows.h>

> The only questionable bit in my opinion is the GTK implementation switches the list store image column type to CAIRO_GOBJECT_TYPE_SURFACE from GDK_TYPE_PIXBUF. Not only does this require the <cairo/cairo-gobject.h> header (which is included with my default libgtk-3-dev installation),

If this may be missing, there may be a need for a more complex check
with a __has_include(<cairo/cairo-gobject.h>)

> but it also performs a copy for each pixbuf to be included in the list. This copy is necessary because Cairo surfaces store device scale, not GDK Pixbufs[2]. The list store takes ownership of the surface (it’s a boxed pointer), and will free it when done[3]. I was not able to figure out how to create a Cairo surface from an incoming pixbuf, g_object_ref() it, and pass it to the list store directly (thus avoiding copies and a premature deallocation).

I was originally unconcerned but this is a new allocation for each row
that shows an image. Probably not too bad in normal use.

Neil

Mitchell

unread,
May 12, 2025, 9:35:34 AMMay 12
to scintilla...@googlegroups.com
Hi Neil,

> On May 11, 2025, at 11:31 PM, Neil Hodgson <scintil...@gmail.com> wrote:
>
> Hi Mitchell,
>
>> Attached is a patch and zip against Scintilla 5.5.6 that implements ...
>
> There are some warnings from compilers and linters so I made some more
> changes that silenced these.

Thanks. I’m not sure why my compiler didn’t catch some of those.

>> The only questionable bit in my opinion is the GTK implementation switches the list store image column type to CAIRO_GOBJECT_TYPE_SURFACE from GDK_TYPE_PIXBUF. Not only does this require the <cairo/cairo-gobject.h> header (which is included with my default libgtk-3-dev installation),
>
> If this may be missing, there may be a need for a more complex check
> with a __has_include(<cairo/cairo-gobject.h>)

I figured if my libgtk-3-dev includes it, perhaps all or most distributions have it too. You know more about GTK than me though.

>> but it also performs a copy for each pixbuf to be included in the list. This copy is necessary because Cairo surfaces store device scale, not GDK Pixbufs[2]. The list store takes ownership of the surface (it’s a boxed pointer), and will free it when done[3]. I was not able to figure out how to create a Cairo surface from an incoming pixbuf, g_object_ref() it, and pass it to the list store directly (thus avoiding copies and a premature deallocation).
>
> I was originally unconcerned but this is a new allocation for each row
> that shows an image. Probably not too bad in normal use.

In my testing with language servers that can return over 100 autocompletion items for me to show, I haven’t noticed any issues. I was initially concerned about the allocations and tried hard to avoid them, but I don’t know enough about GTK, Cairo, and GObject to keep a single copy of a Cairo surface around long enough; those surfaces were constantly being deallocated and my images were being drawn corrupted. I learned to live with the allocations and found that it wasn’t noticeable at all.

Cheers,
Mitchell

Neil

unread,
May 13, 2025, 2:00:26 AMMay 13
to scintilla-interest

Mitchell

unread,
May 13, 2025, 10:33:47 AMMay 13
to scintilla...@googlegroups.com
Hi Neil,

> On May 13, 2025, at 2:00 AM, Neil <scintil...@gmail.com> wrote:
>
> https://sourceforge.net/p/scintilla/code/ci/be457b9f192527a9a6f256f58ecbfdbcd7ebd742/

Thank you!

Cheers,
Mitchell
Reply all
Reply to author
Forward
0 new messages