bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 890 bytes --]

Hi, this issue has been raised many times in reddit and other forums.
Support for hidpi (e.g. my screen is 3000x2000) is rather deficient.
Buttons, checkboxes and other widgets look minuscule. The decorations in
the Fringe are barely visible. I'm not sure whether the best strategy is to
offer manually scaled up pixmap variants or to automatically scale them up
even if the result ends up being a bit blurry. In any case, most screens in
sell today are mid or hi resolution, it's almost impossible to buy a decent
piece of new hardware with a good old 1366x768 screen. And emacs should
catchup.

I propose to make an inventory of all the things that should be fixed
towards hidpi support and maybe provide an external package patching
whatever could be patched as a temporary measure.

I'm using emacs 26.3 in Ubuntu 19.04. My DE is Gnome 3.32, scaling factor =
2.

Best regards
--
Carlos

[-- Attachment #2: Type: text/html, Size: 1109 bytes --]

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Thu, 10 Oct 2019 03:28:55 -0300
> 
> I propose to make an inventory of all the things that should be fixed towards hidpi support and maybe provide
> an external package patching whatever could be patched as a temporary measure.

Please go ahead with making such an inventory, and thanks.

> I'm using emacs 26.3 in Ubuntu 19.04. My DE is Gnome 3.32, scaling factor = 2.

Please also verify that the situation hasn't changed in Emacs 27.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Robert Pluim]

>>>>> On Thu, 10 Oct 2019 11:12:50 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Carlos Pita <carlosjosepita@gmail.com>
    >> Date: Thu, 10 Oct 2019 03:28:55 -0300
    >> 
    >> I propose to make an inventory of all the things that should be fixed towards hidpi support and maybe provide
    >> an external package patching whatever could be patched as a temporary measure.

    Eli> Please go ahead with making such an inventory, and thanks.

    >> I'm using emacs 26.3 in Ubuntu 19.04. My DE is Gnome 3.32, scaling factor = 2.

Is that a font scaling factor of 2 via the gnome-tweak-tool, or a 200%
scale factor via the 'display' settings? I have the latter, and
everything looks ok for me.

Robert

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 585 bytes --]

Hi Eli,

> Please go ahead with making such an inventory, and thanks.

I've attached some cases. In general, this seems to be related to
hardcoded pixmaps. One option is to support at lease 1x, 1.5x and 2x
version, using the one that best accommodates to current resolution
(1.5x screens 1920x1080 are quite popular these days). Another options
is to patch the underlying renderer of these pixmaps so that it scales
them although I don't know if there is one single place to patch here.

> Please also verify that the situation hasn't changed in Emacs 27.

Yes, it's exactly the same.

[-- Attachment #2: Expand widget.png --]
[-- Type: image/png, Size: 7572 bytes --]

[-- Attachment #3: Flymake fringe indicator.png --]
[-- Type: image/png, Size: 926 bytes --]

[-- Attachment #4: Fringe wrap indicator.png --]
[-- Type: image/png, Size: 1612 bytes --]

[-- Attachment #5: Checkbox icons.png --]
[-- Type: image/png, Size: 87823 bytes --]

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

> Is that a font scaling factor of 2 via the gnome-tweak-tool, or a 200%
> scale factor via the 'display' settings? I have the latter, and
> everything looks ok for me.

No font scaling factor, just the display scaling factor. Also, no
fractional scaling factor enabled.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Robert Pluim]

>>>>> On Thu, 10 Oct 2019 10:37:37 -0300, Carlos Pita <carlosjosepita@gmail.com> said:

    >> Is that a font scaling factor of 2 via the gnome-tweak-tool, or a 200%
    >> scale factor via the 'display' settings? I have the latter, and
    >> everything looks ok for me.

    Carlos> No font scaling factor, just the display scaling factor. Also, no
    Carlos> fractional scaling factor enabled.

Youʼre on a much more recent version of GTK than I am, so I suspect
that the way to query the display's scale factor has changed (again),
so weʼre not applying it.

Can you run emacs under gdb, put a breakpoint on 'xg_get_scale', and
see what it returns? (perhaps I should add an internal variable to
expose that, but I worry that people will abuse it).

Robert

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Robert Pluim]

>>>>> On Thu, 10 Oct 2019 10:36:26 -0300, Carlos Pita <carlosjosepita@gmail.com> said:

    Carlos> Hi Eli,
    >> Please go ahead with making such an inventory, and thanks.

    Carlos> I've attached some cases. In general, this seems to be related to
    Carlos> hardcoded pixmaps. One option is to support at lease 1x, 1.5x and 2x
    Carlos> version, using the one that best accommodates to current resolution
    Carlos> (1.5x screens 1920x1080 are quite popular these days). Another options
    Carlos> is to patch the underlying renderer of these pixmaps so that it scales
    Carlos> them although I don't know if there is one single place to patch here.

Perhaps we should convert those pixmaps to some scaleable format, and
then autoscale them?

Robert

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

> Perhaps we should convert those pixmaps to some scaleable format, and
> then autoscale them?

That could be done, but does this render you request for me to debug
the scaling factor issue obsolete? Because it seems to imply that
these particular problems won't be fixed by a correct detection of
scale anyway, and pretty much everything else looks ok here.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

Another option is to provide pixmaps at 1x and 2x. Scaling is trivial
at 2x and any of them will look reasonable well at 1.5x.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  37689@debbugs.gnu.org
> Date: Thu, 10 Oct 2019 16:21:16 +0200
> 
> Perhaps we should convert those pixmaps to some scaleable format, and
> then autoscale them?

You mean, SVG?  We could do that, but we'd like images to display well
even if Emacs cannot display SVG.

Also, I think fringe bitmaps and other icons we use in the UI cannot
be scalable, we need to scale them ourselves.  Not that I'm an expert
on that (so don't take my words as definitive and/or final).

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Thu, 10 Oct 2019 11:33:34 -0300
> Cc: Eli Zaretskii <eliz@gnu.org>, 37689@debbugs.gnu.org
> 
> > Perhaps we should convert those pixmaps to some scaleable format, and
> > then autoscale them?
> 
> That could be done, but does this render you request for me to debug
> the scaling factor issue obsolete?

No, please go ahead with debugging this, and thanks.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

> No, please go ahead with debugging this, and thanks.

Ok, no problem, it looks fine to me:

3701   return scroll_bar_width_for_theme * xg_get_scale (f);
Value returned is $1 = 2

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

On Thu, Oct 10, 2019 at 12:05 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Robert Pluim <rpluim@gmail.com>
> > Cc: Eli Zaretskii <eliz@gnu.org>,  37689@debbugs.gnu.org
> > Date: Thu, 10 Oct 2019 16:21:16 +0200
> >
> > Perhaps we should convert those pixmaps to some scaleable format, and
> > then autoscale them?
>
> You mean, SVG?  We could do that, but we'd like images to display well
> even if Emacs cannot display SVG.
>
> Also, I think fringe bitmaps and other icons we use in the UI cannot
> be scalable, we need to scale them ourselves.  Not that I'm an expert
> on that (so don't take my words as definitive and/or final).

At least for the fringe part, what do you think of modifying
define-fringe-bitmap to take into account scaling factor?

For example, if scaling factor > 1.5 make everything x2, if > 2.5 then
x3, etc. It seems quite simple to achieve those integer scalings.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

> For example, if scaling factor > 1.5 make everything x2, if > 2.5 then
> x3, etc. It seems quite simple to achieve those integer scalings.

Or maybe, safer:

1. >= 3  then  3x
2. >= 2  then  2x
3. Otherwise 1x

This way you are sure that the icon will fit the fringe. And anyway
intermediate resolutions won't usually be 1.9x but 1.5x  instead
(although experimental fractional scaling configuration dialogs
currently offer 1.25x and 1.75x).

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Thu, 10 Oct 2019 12:51:57 -0300
> Cc: Robert Pluim <rpluim@gmail.com>, 37689@debbugs.gnu.org
> 
> At least for the fringe part, what do you think of modifying
> define-fringe-bitmap to take into account scaling factor?

Wouldn't that make the pixels show?

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

> Wouldn't that make the pixels show?

Not more than in a low-dpi screen at 1x, since 4 pixels in a retina
like screen are more or less the same size than 1 "traditional" pixel.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

I tried to hack around this problem with an advice:

(defun my-define-fringe-bitmap-advice (fun bitmap bits &optional
height width align)
  (when (<= (or width 8) 8)
    (setq width (* (or width 8) 2)
          height (when height (* height 2))
          bits (vconcat (mapcan
                         (lambda (in)
                           (let ((out 0))
                             (dotimes (i 8 (list out out))
                               (setq out (+ out (lsh (* (logand in 1)
3) (* i 2)))
                                     in (/ in 2)))))
                         bits))))
  (funcall fun bitmap bits height width align))
(advice-add #'define-fringe-bitmap :around #'my-define-fringe-bitmap-advice)

It works well but too late I realized that many pixmaps are built-in
and don't go through define-fringe-bitmap at all :(.

Maybe it could still be useful as a prototype.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

If you accept to recompute the pixmap on-the-fly each time it's demanded, then

fringe.c:
   static struct fringe_bitmap *get_fringe_bitmap_data (int bn)

seems like a good place to do the transformation above.

In order to avoid rescaling it each time, the pixmap structure could
contain an extra field with its scaling factor (initially 1) that is
updated after the first rescaling so that get_fringe_bitmap_data knows
(by comparing this number to the system scaling factor) that it's not
necessary to rescale the pixmap afterwards.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

> If you accept to recompute the pixmap on-the-fly each time

I've been thinking that in a multi-monitor / multi-dpi world this
(though maybe coupled with some caching strategy, as I proposed in my
previous email) might be the only sensible approach.

If you agree I might try implementing something like what my advice
above does, but inside get_fringe_bitmap_data, with caching and
generalizing it to more scaling factors; nevertheless, I would be
keeping the integer scaling simplification since fractional scaling,
specially for such tiny bitmaps, would require a lot of handcrafting.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Fri, 11 Oct 2019 21:51:02 -0300
> Cc: Robert Pluim <rpluim@gmail.com>, 37689@debbugs.gnu.org
> 
> If you agree I might try implementing something like what my advice
> above does, but inside get_fringe_bitmap_data, with caching and
> generalizing it to more scaling factors; nevertheless, I would be
> keeping the integer scaling simplification since fractional scaling,
> specially for such tiny bitmaps, would require a lot of handcrafting.

Fine with me, thanks.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

Hi Eli,

after debugging the C part of the code I've changed my mind regarding
what I'd said.

First, I realized that the default build wasn't using cairo, so I changed that.

Now I'm exploring how to change scale in xterm.c/x_cr_draw_image, I've
tried changing the scale of the cairo context, of the surface and of
the pattern, yet to no avail. I guess I will have to sit  down and
read about how all these parts interact instead of my current
hit-and-miss approach. But maybe this will fix other problems, for
example those small widgets in my screenshots.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 526 bytes --]

Ok, I have arrived at something, I still have to understand the
details but many things are correctly scaled, including widget images.

Icons in the fringe seems to be ok but yet:
1. So checkboxes are weirdly clipped, even if correctly scaled.
2. Not sure about this, but I believe there is a slight vertical misalignment.
3. Default fringe sizes are too small for hidpi screen, they should
change according to screen dpi.

I'm attaching a diff so that you can have an idea of what I am
changing and probably improve on that.

[-- Attachment #2: scale.diff --]
[-- Type: text/x-patch, Size: 808 bytes --]

diff --git a/src/xterm.c b/src/xterm.c
index b49c9d6..c07bced 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -608,7 +608,9 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_pattern_t *image,
 		 int dest_x, int dest_y, bool overlay_p)
 {
   cairo_t *cr = x_begin_cr_clip (f, gc);
-
+  int scale = xg_get_scale(f);
+  width *= scale;
+  height *= scale;
   if (overlay_p)
     cairo_rectangle (cr, dest_x, dest_y, width, height);
   else
@@ -622,6 +624,7 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_pattern_t *image,
 
   cairo_surface_t *surface;
   cairo_pattern_get_surface (image, &surface);
+  cairo_surface_set_device_scale (surface, 1. / scale, 1. / scale);
   cairo_format_t format = cairo_image_surface_get_format (surface);
   if (format != CAIRO_FORMAT_A8 && format != CAIRO_FORMAT_A1)
     {

[-- Attachment #3: Screenshot from 2019-10-12 05-25-09.png --]
[-- Type: image/png, Size: 23505 bytes --]

[-- Attachment #4: Screenshot from 2019-10-12 05-26-09.png --]
[-- Type: image/png, Size: 5172 bytes --]

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

I've been reading some related code (fringe.c, xterm.c, etc) but at
this point I feel I need some feedback from you in order to advance:

1. In my previous posts I have open two alternative paths:

1.a. Do the scaling as downstream as possible. The problem with this
approach is that I will be fixing the problem only for one backend
(for example, Cairo). Also, I would have to "hack" some assumptions
done upstream (for example, to adjust x, y and scale of a rectangle
that is assumed to be of a different geometry by the caller). The
advantage of the approach is that not only fringe icons but any other
image that is rendered by the lowest level routine will be properly
scaled; but, that said, I noticed that in emacs 26.3 widgets are not
using x_cr_draw_image.

1.b. Do the scaling upstream (for example, in get_fringe_bitmap_data
as proposed above). One problem with this approach is that some
backend could already be scaling output itself (for example, by using
a toolkit that automatically scales according to the device
resolution... do you know if this is the case for windows, for
macos?). Also, it won't fix the widgets issue (anyway, as I said,
neither the "downstream" approach will do it in 26.3).

2. I'm clueless regarding were widgets (I mean checkboxes and things
like that) are rendered. With Cairo backend enabled, x_cr_draw_image
is never reached in 26.3, its only user is the fringe module, I've
checked this in the debugger and by inspecting the code. Not sure
about 27, since tweaking x_cr_draw_image did have a (weird) effect, as
the screenshots in my previous post show.

Any help or opinion regarding these issues will be much appreciated, I
really want to move this forward.

Best regards
--
Carlos

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Sun, 13 Oct 2019 21:40:14 -0300
> Cc: Robert Pluim <rpluim@gmail.com>, 37689@debbugs.gnu.org
> 
> 1.a. Do the scaling as downstream as possible. The problem with this
> approach is that I will be fixing the problem only for one backend
> (for example, Cairo). Also, I would have to "hack" some assumptions
> done upstream (for example, to adjust x, y and scale of a rectangle
> that is assumed to be of a different geometry by the caller). The
> advantage of the approach is that not only fringe icons but any other
> image that is rendered by the lowest level routine will be properly
> scaled; but, that said, I noticed that in emacs 26.3 widgets are not
> using x_cr_draw_image.
> 
> 1.b. Do the scaling upstream (for example, in get_fringe_bitmap_data
> as proposed above). One problem with this approach is that some
> backend could already be scaling output itself (for example, by using
> a toolkit that automatically scales according to the device
> resolution... do you know if this is the case for windows, for
> macos?). Also, it won't fix the widgets issue (anyway, as I said,
> neither the "downstream" approach will do it in 26.3).

Granted, I prefer the second approach.  We should do as little code
duplication as possible.

I don't think individual backends do any scaling, but if some do, it
should be easy to disable the scaling in our code for those backends.

> 2. I'm clueless regarding were widgets (I mean checkboxes and things
> like that) are rendered. With Cairo backend enabled, x_cr_draw_image
> is never reached in 26.3, its only user is the fringe module, I've
> checked this in the debugger and by inspecting the code. Not sure
> about 27, since tweaking x_cr_draw_image did have a (weird) effect, as
> the screenshots in my previous post show.

I'm not sure I understand how this is related to the issue at hand.
Can you elaborate?

Also, what exactly do you mean by "rendered"?  In Emacs, there are
generally 2 stages of displaying any "display element" (a character,
an image, etc.): first, a backend-independent step of loading the
display element, determining its metrics, and performing the display
layout calculations derived from that; and then backend-dependent step
of actually delivering the display element to the glass.  Which one of
those did you have in mind?

Thanks.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Alan Third]

On Mon, Oct 14, 2019 at 11:33:02AM +0300, Eli Zaretskii wrote:
> > From: Carlos Pita <carlosjosepita@gmail.com>
> > Date: Sun, 13 Oct 2019 21:40:14 -0300
> > Cc: Robert Pluim <rpluim@gmail.com>, 37689@debbugs.gnu.org
> > 
> > 1.b. Do the scaling upstream (for example, in get_fringe_bitmap_data
> > as proposed above). One problem with this approach is that some
> > backend could already be scaling output itself (for example, by using
> > a toolkit that automatically scales according to the device
> > resolution... do you know if this is the case for windows, for
> > macos?). Also, it won't fix the widgets issue (anyway, as I said,
> > neither the "downstream" approach will do it in 26.3).
> 
> Granted, I prefer the second approach.  We should do as little code
> duplication as possible.
> 
> I don't think individual backends do any scaling, but if some do, it
> should be easy to disable the scaling in our code for those backends.

macOS automatically scales, so the UI code generally doesn’t need to
know that it’s running on a hi‐DPI screen. The only exception is
images where ideally the program presents an image that matches the
physical DPI of the screen, but the rest of the UI code behaves as if
the screen is half the DPI.

I think it should be easy to make it do the right thing here.
-- 
Alan Third

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> Date: Mon, 14 Oct 2019 14:19:55 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: Carlos Pita <carlosjosepita@gmail.com>, rpluim@gmail.com,
> 	37689@debbugs.gnu.org
> 
> macOS automatically scales, so the UI code generally doesn’t need to
> know that it’s running on a hi‐DPI screen.

Do the fringe bitmaps look good after scaling?

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

> Granted, I prefer the second approach.  We should do as little code duplication as possible.

But we might be duplicating what backends already do.

> I don't think individual backends do any scaling

My understanding is that nowadays everything is up-scaled by the
toolkit, except for fonts that use more sophisticated algorithms to
fit the larger grid of available pixels. So an application using a
modern toolkit should be mostly unaware of screen resolution. In fact,
I've reported some hidpi bugs here and there and, in general, they
were in places where the application did make some explicit geometry
computation based on actual resolution. Or see how "supporting hidpi"
translates in many cases to "port from gtk2 to gtk3". Or look at my
screenshots above, do they look too big? Well, that's because they're
taken from a hidpi screen; now, you might be in a lowdpi one and then
it's obvious that they will look twice as big there; but even if
you're in a hidpi screen I bet you probably will see them doubling the
real thing, because many apps are unaware or ignore the original image
resolution and just let the toolkit show it at 2x. There are plenty of
questions (for both Linux and MacOS) around the web asking "why my
screenshots look too big?", as a cursory google search will show,
although I think the problem was eventually addressed in MacOS,
perhaps in core apps or perhaps in general. That might be the reason
why Alan says that "The only exception is images".

> I think it should be easy to make it do the right thing here.

If you had one single backend this would clearly simplify matters. But
when you had to support three ones, it isn't that clear. Nevertheless,
I think any approach that relies on emacs doing the scaling must be
*carefully* evaluated, because it would mean solving a hard problem
that even toolkits have a hard time to address. Consider for example
having different frames in different monitors each one with its own
dpi and geometry. Are you sure that all geometry/layout computations
for a frame are done by emacs on-the-fly so as they adapt when the
frame is moved from a monitor to another one? Is emacs doing all
geometry calculations itself from the actual device geometry and
resolution or is it most of a hodgepodge, with some things taken care
by the backend and other ones by emacs itself? How clear-cut is the
separation between the stages you mentioned (geometry calculation by
emacs and actual "delivery to the glass" by the backend)? Is it that
clear-cut for *all* supported backends?

Now, that said, I don't think it's a bad idea for emacs to deal with
these matters regardless of its backends (assuming it can force all
backends to work at 1x), since it provides its own toolkit (and its
own OS ;) ). What I would like to know is how close emacs is currently
to one and to the other approach.

> > 2. I'm clueless regarding were widgets (I mean checkboxes and things like that) are rendered.

> I'm not sure I understand how this is related to the issue at hand. Can you elaborate?

By widgets I was referring to the checkboxes, arrows and stuff that
you can see, for example, in a customize-face buffer. When changing
the scaling parameters in x_cr_render_image in emacs 26.3, fringe
bitmaps were affected but those aforementioned widgets weren't. In
emacs 27 they indeed were affected by the same changes in code, but
they looked weirdly distorted and clipped, as you can see in my
previous screenshot. So, in brief, I couldn't locate the C code path
for the rendering of this stuff, specially in emacs 26.3. And by
rendering I was indistinctly referring to both stages you describe.

Best regards
--
Carlos

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Mon, 14 Oct 2019 11:37:03 -0300
> Cc: Eli Zaretskii <eliz@gnu.org>, Robert Pluim <rpluim@gmail.com>, 37689@debbugs.gnu.org
> 
> > > 2. I'm clueless regarding were widgets (I mean checkboxes and things like that) are rendered.
> 
> > I'm not sure I understand how this is related to the issue at hand. Can you elaborate?
> 
> By widgets I was referring to the checkboxes, arrows and stuff that
> you can see, for example, in a customize-face buffer. When changing
> the scaling parameters in x_cr_render_image in emacs 26.3, fringe
> bitmaps were affected but those aforementioned widgets weren't. In
> emacs 27 they indeed were affected by the same changes in code, but
> they looked weirdly distorted and clipped, as you can see in my
> previous screenshot. So, in brief, I couldn't locate the C code path
> for the rendering of this stuff, specially in emacs 26.3. And by
> rendering I was indistinctly referring to both stages you describe.

Maybe I'm confused: I thought we were talking about fringe bitmaps.
But you seem to be talking about a more general issue.  I'm not sure
all of the graphic elements we show on our display should share the
same solution wrt hidpi; in particular, I think fonts don't need
anything to support that.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

> Maybe I'm confused: I thought we were talking about fringe bitmaps.
> But you seem to be talking about a more general issue.  I'm not sure

Yes, remember how all this started:

> make an inventory of all the things that should be fixed towards hidpi

Above I've been mostly talking about two things: i. fringe bitmaps and
ii. widgets (checkbox-like stuff). My main interest is in (i) since
I'm ok with text-only widgets but there is no replacement for fringe
bitmaps. Anyway, I would like to fix both things at the same time.
Moreover, there are other problems, which might be addressed by the
same changes or not, take for example tetris, which looks laughably
small in my screen.

So I could go and  change get_fringe_bitmap_data and hopefully fix the
fringe issue but:

i. This won't fix any other hidpi issues.
ii. This might introduce regressions in backends that I'm unable to
test (windows, macos) because they might be doing the scaling
themselves.

Therefore I believe a more nuanced understanding of how emacs is
approaching the matter, if there is any strategy at all, is in order.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Mon, 14 Oct 2019 12:06:45 -0300
> Cc: Alan Third <alan@idiocy.org>, Robert Pluim <rpluim@gmail.com>, 37689@debbugs.gnu.org
> 
> Above I've been mostly talking about two things: i. fringe bitmaps and
> ii. widgets (checkbox-like stuff).

What you call "widgets" are images.  Fringes are also images, but
their format is fixed: they are always bitmaps.

> i. This won't fix any other hidpi issues.
> ii. This might introduce regressions in backends that I'm unable to
> test (windows, macos) because they might be doing the scaling
> themselves.
> 
> Therefore I believe a more nuanced understanding of how emacs is
> approaching the matter, if there is any strategy at all, is in order.

I think we covered all that, what is left is coding.  Right?

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

> What you call "widgets" are images.  Fringes are also images, but
their format is fixed: they are always bitmaps.

> I think we covered all that, what is left is coding.  Right?

Well, I know they're images, I even known which images they are, I
just haven't spotted the place where they're actually dealt with in
the low level code and I was surprised that, being images, changes to
x_cr_render_image weren't having any effect on them (with the cairo
backend enabled, of course). To add to my confusion there are the
aforementioned differences between 26.3 and 27 in this regard. The
question of which code is dealing with these images (as opposed to
fringe bitmaps) was indeed left unanswered but, nevermind, I'll keep
looking for it myself. Any additional hint would be much appreciated,
though.

For the time being I will focus on fringe bitmaps and work under this
assumption (which I'm not sure is that mild as you seem to suggest):

> I don't think individual backends do any scaling, but if some do, it
> should be easy to disable the scaling in our code for those backends.

Later we can tackle "widgets" (which is the right name for them? They
are indeed defined in widget.el and wid-edit.el AFAICS).

Best regards
--
Carlos

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Mon, 14 Oct 2019 12:32:58 -0300
> Cc: Alan Third <alan@idiocy.org>, Robert Pluim <rpluim@gmail.com>, 37689@debbugs.gnu.org
> 
> > What you call "widgets" are images.  Fringes are also images, but
> their format is fixed: they are always bitmaps.
> 
> > I think we covered all that, what is left is coding.  Right?
> 
> Well, I know they're images, I even known which images they are, I
> just haven't spotted the place where they're actually dealt with in
> the low level code

The image-handling code is in image.c.  The display engine in xdisp.c
uses that to perform layout calculations, and then the backends
actually display the images, e.g. look at x_draw_image_glyph_string
and its subroutines.  Fringe bitmaps use separate backend-dependent
code for the actual display, see x_draw_fringe_bitmap as one example.

> Later we can tackle "widgets" (which is the right name for them?

I suggest "images".

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

Ok, while working on this I bumped into a new problem and I'm stuck again.

In order to scale things "upstream" I need the scaling factor, but the
functions that compute the scaling factor are static functions defined
"downstream" in each backend: x_get_scale_factor in w32term.c, in
xterm.c. Or, in other places, some code that is conditional to GTK
simply calls xg_get_scale.

Do you know of any backend-agnostic way to get the scaling factor for
a frame? I don't want to replicate complex backend dependant code in
fringe.c.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

I'm finding non-trivial difficulties that make me think this is not
the best approach.

I've already mentioned one above:

1. I need the scaling factor in fringe.c yet it's not part of the
redisplay_interface. Functions that compute scaling factor are backend
specific and static.

Also:

2. init_fringe_bitmap does non-trivial manipulations to the original
bit sequence (nibble swapping, shifting, casting) to produce a
platform/backend-specific representation. redisplay_interface is
ill-defined in this regard, bitmap representation is not part of the
interface. For some backends init_fringe_bitmap even compresses shorts
to chars if width < 8, so I can't reliably detect row limits in a
platform/backend-agnostic way, which I need in order to scale the
bitmap.

3. The unsigned short representation is unfortunate. For 3x we need at
least int64_t. Then we need to modify all that backend-dependent
nibble swapping, shifting and casting that gives me the creeps.
Finally backends would have to be adapted to take an int64_t array as
input.

Given those considerable difficulties, I propose to scale bitmaps in
two stages instead:

a. At the level of fringe.c we only modify the geometry (width,
height) of the image, so that calculations that are independent of the
bitmap itself are correctly done. This way we can keep the unsigned
short representation, we don't need to touch that complex
platform/backend-dependent bit and we don't need to query the scaling
factor, thus solving points 1, 2 and 3 above.

b. Then each backend should set a transformation matrix or something
like that so that the bitmap is scaled to the appropriate resolution.
I already know how to do that for Cairo, it's trivial.

Eli, what do you think?

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

> factor, thus solving points 1, 2 and 3 above.

Sorry, problem 1 is still unsolved by the proposed approach, not
without extending redisplay_interface.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

> > factor, thus solving points 1, 2 and 3 above.
>
> Sorry, problem 1 is still unsolved by the proposed approach, not
> without extending redisplay_interface.

Also, the scaling factor should probably be exposed to frame.c, since
it sets a default width of 8 that assumes a standard dpi monitor:

gui_set_left/right_fringe (struct frame *f, Lisp_Object new_value,
Lisp_Object old_value)
  ...
  new_width = (RANGED_FIXNUMP (-INT_MAX, new_value, INT_MAX)
       ? eabs (XFIXNUM (new_value)) : 8);

I've reached a point from where I'm unable to continue ahead on my
own, except for writing a private patch targeting my specific
resolution and backend. I would like to contribute but I see no way
without extending the interface and that's far beyond than what I can
decide.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Mon, 14 Oct 2019 20:42:59 -0300
> Cc: Alan Third <alan@idiocy.org>, Robert Pluim <rpluim@gmail.com>, 37689@debbugs.gnu.org
> 
> I'm finding non-trivial difficulties that make me think this is not
> the best approach.
> 
> I've already mentioned one above:
> 
> 1. I need the scaling factor in fringe.c yet it's not part of the
> redisplay_interface. Functions that compute scaling factor are backend
> specific and static.

Adding them to frame redisplay interface would solve this.

> 2. init_fringe_bitmap does non-trivial manipulations to the original
> bit sequence (nibble swapping, shifting, casting) to produce a
> platform/backend-specific representation. redisplay_interface is
> ill-defined in this regard, bitmap representation is not part of the
> interface. For some backends init_fringe_bitmap even compresses shorts
> to chars if width < 8, so I can't reliably detect row limits in a
> platform/backend-agnostic way, which I need in order to scale the
> bitmap.

The code in init_fringe_bitmap obviously need some refactoring to rely
on redisplay interface.  You could make such refactoring part of the
job, or you could leave the current code intact and use its results.
I don't think I understand the difficulty of detecting row limits, but
you could begin by doing that for one platform, and then asking others
to do the same for other platforms.

> 3. The unsigned short representation is unfortunate. For 3x we need at
> least int64_t. Then we need to modify all that backend-dependent
> nibble swapping, shifting and casting that gives me the creeps.
> Finally backends would have to be adapted to take an int64_t array as
> input.

Couldn't we use the existing image-scaling code for that?  It is
implemented in each backend already.

> a. At the level of fringe.c we only modify the geometry (width,
> height) of the image, so that calculations that are independent of the
> bitmap itself are correctly done. This way we can keep the unsigned
> short representation, we don't need to touch that complex
> platform/backend-dependent bit and we don't need to query the scaling
> factor, thus solving points 1, 2 and 3 above.
> 
> b. Then each backend should set a transformation matrix or something
> like that so that the bitmap is scaled to the appropriate resolution.
> I already know how to do that for Cairo, it's trivial.
> 
> Eli, what do you think?

I don't think I understand what will stage (a) do under this proposal.
Stage (b) is already implemented, you just need to use it, and you
need to tell the transformation code the correct scale factor.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Mon, 14 Oct 2019 22:50:11 -0300
> Cc: Alan Third <alan@idiocy.org>, Robert Pluim <rpluim@gmail.com>, 37689@debbugs.gnu.org
> 
> Also, the scaling factor should probably be exposed to frame.c, since
> it sets a default width of 8 that assumes a standard dpi monitor:
> 
> gui_set_left/right_fringe (struct frame *f, Lisp_Object new_value,
> Lisp_Object old_value)
>   ...
>   new_width = (RANGED_FIXNUMP (-INT_MAX, new_value, INT_MAX)
>        ? eabs (XFIXNUM (new_value)) : 8);
> 
> I've reached a point from where I'm unable to continue ahead on my
> own, except for writing a private patch targeting my specific
> resolution and backend. I would like to contribute but I see no way
> without extending the interface and that's far beyond than what I can
> decide.

Extending the frame redisplay interface is a no-brainer, please go
ahead.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

I decided to follow a more incremental strategy because I feel this is
getting unwieldy.

I split the issue in three parts for now:

1. Expose scale factor in the rif (patch in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37770)
2. Implement image scaling in Cairo and maybe X11 backends (this ticket)
3. Some code cleanups / refactors pertaining the initialization
sequence of backends (patch in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37755)

1 is a must-have for 2, 3 is a nice-to-have.

I'm writing a proper patch for 2 on top of my patch for 1 that I will
be attaching here soon.

Are you ok with this strategy? It's not the one-hack-to-rule-them-all
approach envisioned at the beginning but it simplify things a lot for
anyone wanting to make macos and win backends actually scale their
bitmaps, which shouldn't be difficult at all (or even necessary, I
still have doubts regarding macos). Plus it hopefully brings some code
improvements and a necessary API.

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

Tags: patch

Here is a patch to be applied on top of
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37770.

I tried to postpone scaling calculations whenever possible so as to be
prepared for a multi-monitor/multi-dpi future. Of course, this is at
the expense of fetching scaling factors every time, but I believe it
is a reasonable price to pay.

[-- Attachment #2: 0001-Make-fringe-honour-scale-factor-in-Cairo-backend.patch --]
[-- Type: text/x-patch, Size: 3135 bytes --]

From 0aa5b0339228fa85594de7bb9f78feee938bd7e8 Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Wed, 16 Oct 2019 01:18:49 -0300
Subject: [PATCH] Make fringe honour scale factor in Cairo backend

* src/fringe.c: scale bitmap with and height
* src/xterm.c: set device scale.
* sec/window.h: scale WINDOW_LEFT/RIGHT_FRINGE_WIDTH macros
---
 src/fringe.c |  5 +++--
 src/window.h | 14 ++++++++++----
 src/xterm.c  |  2 ++
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/fringe.c b/src/fringe.c
index 22f3bdc..5242e84 100644
--- a/src/fringe.c
+++ b/src/fringe.c
@@ -563,6 +563,7 @@ draw_fringe_bitmap_1 (struct window *w, struct glyph_row *row, int left_p, int o
   int period;
   int face_id = DEFAULT_FACE_ID;
   int offset, header_line_height;
+  double scale = FRAME_RIF (f)->get_scale_factor (f);
 
   p.overlay_p = (overlay & 1) == 1;
   p.cursor_p = (overlay & 2) == 2;
@@ -602,9 +603,9 @@ draw_fringe_bitmap_1 (struct window *w, struct glyph_row *row, int left_p, int o
 
   p.which = which;
   p.bits = fb->bits;
-  p.wd = fb->width;
+  p.wd = fb->width * scale;
 
-  p.h = fb->height;
+  p.h = fb->height * scale;
   p.dh = (period > 0 ? (p.y % period) : 0);
   p.h -= p.dh;
 
diff --git a/src/window.h b/src/window.h
index 71946a5..7937d1e 100644
--- a/src/window.h
+++ b/src/window.h
@@ -827,16 +827,22 @@ #define WINDOW_MARGINS_WIDTH(W)			\
   (WINDOW_LEFT_MARGIN_WIDTH (W)			\
    + WINDOW_RIGHT_MARGIN_WIDTH (W))
 
+#define WINDOW_SCALE_FACTOR(W)							\
+  (FRAME_RIF (WINDOW_XFRAME (W)) == 0 ? 1					\
+   : FRAME_RIF (WINDOW_XFRAME (W))->get_scale_factor (WINDOW_XFRAME (W)))
+
 /* Pixel-widths of fringes.  */
 #define WINDOW_LEFT_FRINGE_WIDTH(W)			\
-  (W->left_fringe_width >= 0				\
+  ((W->left_fringe_width >= 0				\
    ? W->left_fringe_width				\
-   : FRAME_LEFT_FRINGE_WIDTH (WINDOW_XFRAME (W)))
+   : FRAME_LEFT_FRINGE_WIDTH (WINDOW_XFRAME (W)))	\
+   * WINDOW_SCALE_FACTOR (W))
 
 #define WINDOW_RIGHT_FRINGE_WIDTH(W)			\
-  (W->right_fringe_width >= 0				\
+  ((W->right_fringe_width >= 0				\
    ? W->right_fringe_width				\
-   : FRAME_RIGHT_FRINGE_WIDTH (WINDOW_XFRAME (W)))
+   : FRAME_RIGHT_FRINGE_WIDTH (WINDOW_XFRAME (W)))	\
+   * WINDOW_SCALE_FACTOR (W))
 
 #define WINDOW_FRINGES_WIDTH(W)		\
   (WINDOW_LEFT_FRINGE_WIDTH (W) + WINDOW_RIGHT_FRINGE_WIDTH (W))
diff --git a/src/xterm.c b/src/xterm.c
index 67253a6..bf1aba2 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -608,6 +608,7 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_pattern_t *image,
 		 int dest_x, int dest_y, bool overlay_p)
 {
   cairo_t *cr = x_begin_cr_clip (f, gc);
+  double scale = FRAME_RIF (f)->get_scale_factor (f);
 
   if (overlay_p)
     cairo_rectangle (cr, dest_x, dest_y, width, height);
@@ -622,6 +623,7 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_pattern_t *image,
 
   cairo_surface_t *surface;
   cairo_pattern_get_surface (image, &surface);
+  cairo_surface_set_device_scale (surface, 1. / scale, 1. / scale);
   cairo_format_t format = cairo_image_surface_get_format (surface);
   if (format != CAIRO_FORMAT_A8 && format != CAIRO_FORMAT_A1)
     {
-- 
2.20.1

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[martin rudalics]

 > +#define WINDOW_SCALE_FACTOR(W)							\

Please just call it WINDOW_FRAME_SCALE_FACTOR or FRAME_SCALE_FACTOR.
WINDOW_SCALE_FACTOR implies that it could be different for different
windows on the same frame.

martin

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]

> Please just call it WINDOW_FRAME_SCALE_FACTOR or FRAME_SCALE_FACTOR.
> WINDOW_SCALE_FACTOR implies that it could be different for different
> windows on the same frame.

Good point, I've changed the name to WINDOW_FRAME_SCALE_FACTOR.

I've a bad feeling towards the conditional hidden behind that macro,
though. It's necessary because WINDOW_LEFT/RIGHT_FRINGE_WIDTH are
being called in contexts where the rif is still unavailable; so
knowing of no other sensible default, I just defaulted to 1. Now,
there is a bad smell in these requests for geometry parameters in
places where the exact geometry may still be unknown (although I
believe geometry is indeed known because, even if there isn't a rif,
cursory debugging shows that there already is a frame in all
problematic cases). So either these usages asume geometry is not
dependent on the current screen or, less problematically and more
probably, the rif is being initialized too late. A careful inspection
of every use place is a lot of work (I might be doing some of it next
weekend, though) so for now I'm just returning a sane default so that
we can safely move forward.

[-- Attachment #2: 0001-Make-fringe-honour-scale-factor-in-Cairo-backend.patch --]
[-- Type: text/x-patch, Size: 3135 bytes --]

From 6ce862360bc9369916a25c933bf4a84b989849b6 Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Wed, 16 Oct 2019 01:18:49 -0300
Subject: [PATCH] Make fringe honour scale factor in Cairo backend

* src/fringe.c: scale bitmap with and height
* src/xterm.c: set device scale.
* sec/window.h: scale WINDOW_LEFT/RIGHT_FRINGE_WIDTH macros
---
 src/fringe.c |  5 +++--
 src/window.h | 14 ++++++++++----
 src/xterm.c  |  2 ++
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/fringe.c b/src/fringe.c
index 22f3bdc..5242e84 100644
--- a/src/fringe.c
+++ b/src/fringe.c
@@ -563,6 +563,7 @@ draw_fringe_bitmap_1 (struct window *w, struct glyph_row *row, int left_p, int o
   int period;
   int face_id = DEFAULT_FACE_ID;
   int offset, header_line_height;
+  double scale = FRAME_RIF (f)->get_scale_factor (f);
 
   p.overlay_p = (overlay & 1) == 1;
   p.cursor_p = (overlay & 2) == 2;
@@ -602,9 +603,9 @@ draw_fringe_bitmap_1 (struct window *w, struct glyph_row *row, int left_p, int o
 
   p.which = which;
   p.bits = fb->bits;
-  p.wd = fb->width;
+  p.wd = fb->width * scale;
 
-  p.h = fb->height;
+  p.h = fb->height * scale;
   p.dh = (period > 0 ? (p.y % period) : 0);
   p.h -= p.dh;
 
diff --git a/src/window.h b/src/window.h
index 71946a5..7937d1e 100644
--- a/src/window.h
+++ b/src/window.h
@@ -827,16 +827,22 @@ #define WINDOW_MARGINS_WIDTH(W)			\
   (WINDOW_LEFT_MARGIN_WIDTH (W)			\
    + WINDOW_RIGHT_MARGIN_WIDTH (W))
 
+#define WINDOW_SCALE_FACTOR(W)							\
+  (FRAME_RIF (WINDOW_XFRAME (W)) == 0 ? 1					\
+   : FRAME_RIF (WINDOW_XFRAME (W))->get_scale_factor (WINDOW_XFRAME (W)))
+
 /* Pixel-widths of fringes.  */
 #define WINDOW_LEFT_FRINGE_WIDTH(W)			\
-  (W->left_fringe_width >= 0				\
+  ((W->left_fringe_width >= 0				\
    ? W->left_fringe_width				\
-   : FRAME_LEFT_FRINGE_WIDTH (WINDOW_XFRAME (W)))
+   : FRAME_LEFT_FRINGE_WIDTH (WINDOW_XFRAME (W)))	\
+   * WINDOW_SCALE_FACTOR (W))
 
 #define WINDOW_RIGHT_FRINGE_WIDTH(W)			\
-  (W->right_fringe_width >= 0				\
+  ((W->right_fringe_width >= 0				\
    ? W->right_fringe_width				\
-   : FRAME_RIGHT_FRINGE_WIDTH (WINDOW_XFRAME (W)))
+   : FRAME_RIGHT_FRINGE_WIDTH (WINDOW_XFRAME (W)))	\
+   * WINDOW_SCALE_FACTOR (W))
 
 #define WINDOW_FRINGES_WIDTH(W)		\
   (WINDOW_LEFT_FRINGE_WIDTH (W) + WINDOW_RIGHT_FRINGE_WIDTH (W))
diff --git a/src/xterm.c b/src/xterm.c
index 67253a6..bf1aba2 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -608,6 +608,7 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_pattern_t *image,
 		 int dest_x, int dest_y, bool overlay_p)
 {
   cairo_t *cr = x_begin_cr_clip (f, gc);
+  double scale = FRAME_RIF (f)->get_scale_factor (f);
 
   if (overlay_p)
     cairo_rectangle (cr, dest_x, dest_y, width, height);
@@ -622,6 +623,7 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_pattern_t *image,
 
   cairo_surface_t *surface;
   cairo_pattern_get_surface (image, &surface);
+  cairo_surface_set_device_scale (surface, 1. / scale, 1. / scale);
   cairo_format_t format = cairo_image_surface_get_format (surface);
   if (format != CAIRO_FORMAT_A8 && format != CAIRO_FORMAT_A1)
     {
-- 
2.20.1

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 32 bytes --]

Sorry, this is the right patch.

[-- Attachment #2: 0001-Make-fringe-honour-scale-factor-in-Cairo-backend.patch --]
[-- Type: text/x-patch, Size: 3144 bytes --]

From 6b8dbc8055c67fea1b01ecf84e9e751f9919e9c5 Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Wed, 16 Oct 2019 01:18:49 -0300
Subject: [PATCH] Make fringe honour scale factor in Cairo backend

* src/fringe.c: scale bitmap with and height
* src/xterm.c: set device scale.
* sec/window.h: scale WINDOW_LEFT/RIGHT_FRINGE_WIDTH macros
---
 src/fringe.c |  5 +++--
 src/window.h | 14 ++++++++++----
 src/xterm.c  |  2 ++
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/fringe.c b/src/fringe.c
index 22f3bdc..5242e84 100644
--- a/src/fringe.c
+++ b/src/fringe.c
@@ -563,6 +563,7 @@ draw_fringe_bitmap_1 (struct window *w, struct glyph_row *row, int left_p, int o
   int period;
   int face_id = DEFAULT_FACE_ID;
   int offset, header_line_height;
+  double scale = FRAME_RIF (f)->get_scale_factor (f);
 
   p.overlay_p = (overlay & 1) == 1;
   p.cursor_p = (overlay & 2) == 2;
@@ -602,9 +603,9 @@ draw_fringe_bitmap_1 (struct window *w, struct glyph_row *row, int left_p, int o
 
   p.which = which;
   p.bits = fb->bits;
-  p.wd = fb->width;
+  p.wd = fb->width * scale;
 
-  p.h = fb->height;
+  p.h = fb->height * scale;
   p.dh = (period > 0 ? (p.y % period) : 0);
   p.h -= p.dh;
 
diff --git a/src/window.h b/src/window.h
index 71946a5..ac66af8 100644
--- a/src/window.h
+++ b/src/window.h
@@ -827,16 +827,22 @@ #define WINDOW_MARGINS_WIDTH(W)			\
   (WINDOW_LEFT_MARGIN_WIDTH (W)			\
    + WINDOW_RIGHT_MARGIN_WIDTH (W))
 
+#define WINDOW_FRAME_SCALE_FACTOR(W)		\
+  (FRAME_RIF (WINDOW_XFRAME (W)) == 0 ? 1	\
+   : FRAME_RIF (WINDOW_XFRAME (W))->get_scale_factor (WINDOW_XFRAME (W)))
+
 /* Pixel-widths of fringes.  */
 #define WINDOW_LEFT_FRINGE_WIDTH(W)			\
-  (W->left_fringe_width >= 0				\
+  ((W->left_fringe_width >= 0				\
    ? W->left_fringe_width				\
-   : FRAME_LEFT_FRINGE_WIDTH (WINDOW_XFRAME (W)))
+   : FRAME_LEFT_FRINGE_WIDTH (WINDOW_XFRAME (W)))	\
+   * WINDOW_FRAME_SCALE_FACTOR (W))
 
 #define WINDOW_RIGHT_FRINGE_WIDTH(W)			\
-  (W->right_fringe_width >= 0				\
+  ((W->right_fringe_width >= 0				\
    ? W->right_fringe_width				\
-   : FRAME_RIGHT_FRINGE_WIDTH (WINDOW_XFRAME (W)))
+   : FRAME_RIGHT_FRINGE_WIDTH (WINDOW_XFRAME (W)))	\
+   * WINDOW_FRAME_SCALE_FACTOR (W))
 
 #define WINDOW_FRINGES_WIDTH(W)		\
   (WINDOW_LEFT_FRINGE_WIDTH (W) + WINDOW_RIGHT_FRINGE_WIDTH (W))
diff --git a/src/xterm.c b/src/xterm.c
index 67253a6..bf1aba2 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -608,6 +608,7 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_pattern_t *image,
 		 int dest_x, int dest_y, bool overlay_p)
 {
   cairo_t *cr = x_begin_cr_clip (f, gc);
+  double scale = FRAME_RIF (f)->get_scale_factor (f);
 
   if (overlay_p)
     cairo_rectangle (cr, dest_x, dest_y, width, height);
@@ -622,6 +623,7 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_pattern_t *image,
 
   cairo_surface_t *surface;
   cairo_pattern_get_surface (image, &surface);
+  cairo_surface_set_device_scale (surface, 1. / scale, 1. / scale);
   cairo_format_t format = cairo_image_surface_get_format (surface);
   if (format != CAIRO_FORMAT_A8 && format != CAIRO_FORMAT_A1)
     {
-- 
2.20.1

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 67 bytes --]

Improved the commit message to hopefully match CONTRIBUTING rules.

[-- Attachment #2: 0001-Make-fringe-honour-scale-factor-in-Cairo-backend-Bug.patch --]
[-- Type: text/x-patch, Size: 3271 bytes --]

From eae482e5103b82642306e55c0c6fc985d46b36bb Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Wed, 16 Oct 2019 01:18:49 -0300
Subject: [PATCH] Make fringe honour scale factor in Cairo backend (Bug#37689)

* src/fringe.c (draw_fringe_bitmap_1): Scale bitmap width and height.
* src/xterm.c (x_cr_draw_image): Set device scale.
* sec/window.h (WINDOW_FRAME_SCALE_FACTOR, WINDOW_LEFT_FRINGE_WIDTH)
(WINDOW_RIGHT_FRINGE_WIDTH): Add helper macro to scale result.
---
 src/fringe.c |  5 +++--
 src/window.h | 14 ++++++++++----
 src/xterm.c  |  2 ++
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/fringe.c b/src/fringe.c
index 22f3bdc..5242e84 100644
--- a/src/fringe.c
+++ b/src/fringe.c
@@ -563,6 +563,7 @@ draw_fringe_bitmap_1 (struct window *w, struct glyph_row *row, int left_p, int o
   int period;
   int face_id = DEFAULT_FACE_ID;
   int offset, header_line_height;
+  double scale = FRAME_RIF (f)->get_scale_factor (f);
 
   p.overlay_p = (overlay & 1) == 1;
   p.cursor_p = (overlay & 2) == 2;
@@ -602,9 +603,9 @@ draw_fringe_bitmap_1 (struct window *w, struct glyph_row *row, int left_p, int o
 
   p.which = which;
   p.bits = fb->bits;
-  p.wd = fb->width;
+  p.wd = fb->width * scale;
 
-  p.h = fb->height;
+  p.h = fb->height * scale;
   p.dh = (period > 0 ? (p.y % period) : 0);
   p.h -= p.dh;
 
diff --git a/src/window.h b/src/window.h
index 71946a5..ac66af8 100644
--- a/src/window.h
+++ b/src/window.h
@@ -827,16 +827,22 @@ #define WINDOW_MARGINS_WIDTH(W)			\
   (WINDOW_LEFT_MARGIN_WIDTH (W)			\
    + WINDOW_RIGHT_MARGIN_WIDTH (W))
 
+#define WINDOW_FRAME_SCALE_FACTOR(W)		\
+  (FRAME_RIF (WINDOW_XFRAME (W)) == 0 ? 1	\
+   : FRAME_RIF (WINDOW_XFRAME (W))->get_scale_factor (WINDOW_XFRAME (W)))
+
 /* Pixel-widths of fringes.  */
 #define WINDOW_LEFT_FRINGE_WIDTH(W)			\
-  (W->left_fringe_width >= 0				\
+  ((W->left_fringe_width >= 0				\
    ? W->left_fringe_width				\
-   : FRAME_LEFT_FRINGE_WIDTH (WINDOW_XFRAME (W)))
+   : FRAME_LEFT_FRINGE_WIDTH (WINDOW_XFRAME (W)))	\
+   * WINDOW_FRAME_SCALE_FACTOR (W))
 
 #define WINDOW_RIGHT_FRINGE_WIDTH(W)			\
-  (W->right_fringe_width >= 0				\
+  ((W->right_fringe_width >= 0				\
    ? W->right_fringe_width				\
-   : FRAME_RIGHT_FRINGE_WIDTH (WINDOW_XFRAME (W)))
+   : FRAME_RIGHT_FRINGE_WIDTH (WINDOW_XFRAME (W)))	\
+   * WINDOW_FRAME_SCALE_FACTOR (W))
 
 #define WINDOW_FRINGES_WIDTH(W)		\
   (WINDOW_LEFT_FRINGE_WIDTH (W) + WINDOW_RIGHT_FRINGE_WIDTH (W))
diff --git a/src/xterm.c b/src/xterm.c
index 67253a6..bf1aba2 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -608,6 +608,7 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_pattern_t *image,
 		 int dest_x, int dest_y, bool overlay_p)
 {
   cairo_t *cr = x_begin_cr_clip (f, gc);
+  double scale = FRAME_RIF (f)->get_scale_factor (f);
 
   if (overlay_p)
     cairo_rectangle (cr, dest_x, dest_y, width, height);
@@ -622,6 +623,7 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_pattern_t *image,
 
   cairo_surface_t *surface;
   cairo_pattern_get_surface (image, &surface);
+  cairo_surface_set_device_scale (surface, 1. / scale, 1. / scale);
   cairo_format_t format = cairo_image_surface_get_format (surface);
   if (format != CAIRO_FORMAT_A8 && format != CAIRO_FORMAT_A1)
     {
-- 
2.20.1

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Robert Pluim]

>>>>> On Wed, 16 Oct 2019 16:01:30 -0300, Carlos Pita <carlosjosepita@gmail.com> said:

    Carlos> Improved the commit message to hopefully match
    Carlos> CONTRIBUTING rules.

Much better. Small comment below:

* sec/window.h (WINDOW_FRAME_SCALE_FACTOR): New helper macro to get
scale factor.
(WINDOW_LEFT_FRINGE_WIDTH,WINDOW_RIGHT_FRINGE_WIDTH): Use it.

(ie split the discussion of the new macro from the description of
where itʼs used).  And itʼs 'src/window.h', which tells me you
probably didnʼt use C-x 4 A or similar :-)

Robert

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Alan Third]

On Mon, Oct 14, 2019 at 05:00:22PM +0300, Eli Zaretskii wrote:
> > Date: Mon, 14 Oct 2019 14:19:55 +0100
> > From: Alan Third <alan@idiocy.org>
> > Cc: Carlos Pita <carlosjosepita@gmail.com>, rpluim@gmail.com,
> > 	37689@debbugs.gnu.org
> > 
> > macOS automatically scales, so the UI code generally doesn’t need to
> > know that it’s running on a hi‐DPI screen.
> 
> Do the fringe bitmaps look good after scaling?

They look absolutely fine to me.
-- 
Alan Third

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Juri Linkov]

>> > Perhaps we should convert those pixmaps to some scaleable format, and
>> > then autoscale them?
>>
>> You mean, SVG?  We could do that, but we'd like images to display well
>> even if Emacs cannot display SVG.
>>
>> Also, I think fringe bitmaps and other icons we use in the UI cannot
>> be scalable, we need to scale them ourselves.  Not that I'm an expert
>> on that (so don't take my words as definitive and/or final).
>
> At least for the fringe part, what do you think of modifying
> define-fringe-bitmap to take into account scaling factor?
>
> For example, if scaling factor > 1.5 make everything x2, if > 2.5 then
> x3, etc. It seems quite simple to achieve those integer scalings.

Funnily enough, the current fringe bitmaps are too big for me,
so I had to customize them using smaller bitmaps copied from somewhere.
I guess it should be possible to do the same to provide x2 scaled bitmaps:

  (define-fringe-bitmap 'light-down-arrow [32 32 32 32 32 32 168 112 32] nil nil 'bottom)
  (define-fringe-bitmap 'light-up-arrow [32 112 168 32 32 32 32 32 32] nil nil 'top)
  (define-fringe-bitmap 'light-top-left-angle [254 254 128 128 128] nil nil 'top)
  (define-fringe-bitmap 'light-bottom-left-angle [128 128 128 254 254] nil  nil 'bottom)
  (define-fringe-bitmap 'light-left-bracket [254 254 128 128 128 0 0 0 0 128 128 128 254 254] nil nil 'center)
  (define-fringe-bitmap 'light-right-curly-arrow [96 16 8 8 72 80 96 120] nil nil 'bottom)
  (define-fringe-bitmap 'light-left-curly-arrow [8 16 16 16 18 10 6 30] nil nil 'top)
  (define-fringe-bitmap 'light-right-arrow [16 8 252 8 16] nil 11 'center)
  (define-fringe-bitmap 'light-left-arrow [32 64 254 64 32] nil nil 'center)
  (setq-default fringe-indicator-alist
                '((truncation . (light-left-arrow light-right-arrow))
                  (continuation . (light-left-curly-arrow light-right-curly-arrow))
                  (overlay-arrow . right-triangle)
                  (up . light-up-arrow)
                  (down . light-down-arrow)
                  (top . (light-top-left-angle top-right-angle))
                  (bottom . (light-bottom-left-angle bottom-right-angle
                             top-right-angle light-top-left-angle))
                  (top-bottom . (light-left-bracket right-bracket
                                 top-right-angle light-top-left-angle))
                  (empty-line . empty-line)
                  (unknown . question-mark)))

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

Hi Juri,

> I guess it should be possible to do the same to provide x2 scaled bitmaps:
>
>   (define-fringe-bitmap 'light-down-arrow [32 32 32 32 32 32 168 112 32] nil nil 'bottom)

This is indeed a cool workaround if you can override the builtin
bitmaps. But anyway it won't address the problem of having different
frames with different dpis nor the need to expose a "scale factor api"
to fix not only this but other hidpi-related issues.

> Funnily enough, the current fringe bitmaps are too big for me,

Too big as in "twice as big as expected" or in "too big for my taste"?

Best regards
--
Carlos

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Juri Linkov]

>> I guess it should be possible to do the same to provide x2 scaled bitmaps:
>>
>>   (define-fringe-bitmap 'light-down-arrow [32 32 32 32 32 32 168 112 32] nil nil 'bottom)
>
> This is indeed a cool workaround if you can override the builtin
> bitmaps.

Actually, this doesn't override the builtin bitmaps.
The name of the builtin bitmap is 'down-arrow',
but this defines a new bitmap 'light-down-arrow'.
You can use any name you want, e.g. 'down-arrow-2x'.

> But anyway it won't address the problem of having different
> frames with different dpis nor the need to expose a "scale factor api"
> to fix not only this but other hidpi-related issues.

Maybe the value of 'fringe-indicator-alist' should be frame-local?
Then depending on the exposed scale-factor, each frame could use
own set of bitmaps defined by frame-local 'fringe-indicator-alist'.

>> Funnily enough, the current fringe bitmaps are too big for me,
>
> Too big as in "twice as big as expected" or in "too big for my taste"?

I use a small 10px font, so the customized bitmaps fit the font size.
So maybe fridge bitmap size should depend on the font size, not scale?

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 1595 bytes --]

>
>
> Maybe the value of 'fringe-indicator-alist' should be frame-local?
>

Frames can be moved from one display to another with a different dpi.


> >> Funnily enough, the current fringe bitmaps are too big for me,
> >
> > Too big as in "twice as big as expected" or in "too big for my taste"?
>
> I use a small 10px font, so the customized bitmaps fit the font size.
>


Ah ok, great, so that's not a scale factor issue.

So maybe fridge bitmap size should depend on the font size, not scale?
>

10px fonts are not 10px at other scale factor than 1x. In general you don't
have to change the size of your fonts when duplicating your screen
resolution, even if your fonts were given in px size, because the toolkit
scales them for you under the assumption of some standard ~96dpi base which
allows pixels to be treated as something more than a "number of tiny dots,
whatever their size is" measure.

What can be done is to adjust everything else in emacs to the effective
(not nominal) pixel size of the default face (then scale factor would be
the effective to nominal pixel size ratio of this font). I believe
something like that is done in some places, it's sensible given that emacs
is mostly a grid of characters. But anyway this is clearly not the approach
taken for the fringe and some parts of emacs have geometry not so tightly
coupled to font size, although I would indeed expect high correlation. And
some toolkits (for example, gtk) offer a separate scale factor for font
size, which is then applied on top of the general scale factor and seen
mostly as an accessibility feature.

>

[-- Attachment #2: Type: text/html, Size: 2789 bytes --]

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Carlos Pita]

I'm closing this issue in order to prevent wasting your time in
useless reviews, because I'm working in a rather different solution as
described in my last comment to #37770. Hopefully I will be posting a
new patch as soon as I get that working, but it has shown to be a
difficult task to undertake... Nevertheless I'm learning a lot about
the internals.

Best regards
--
Carlos

bug#37689: Fringe pixmaps, widgets, etc. look ridiculously tiny in hidpi screen

[Eli Zaretskii]

> From: Carlos Pita <carlosjosepita@gmail.com>
> Date: Thu, 24 Oct 2019 14:09:44 -0300
> Cc: Eli Zaretskii <eliz@gnu.org>, Robert Pluim <rpluim@gmail.com>, 37689-close@debbugs.gnu.org
> 
> I'm closing this issue in order to prevent wasting your time in
> useless reviews, because I'm working in a rather different solution as
> described in my last comment to #37770.

Thanks, but what about bug#37755?  Should we still discuss the changes
there?  Are they independent of what you are working on in bug#37770?