GNU bug report logs - #37932
[PATCH] Support hidpi fringes and images with Cairo

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 37932 in the body.
You can then email your comments to 37932 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Support hidpi fringes and images with Cairo
Date: Fri, 25 Oct 2019 23:17:43 -0300

[Message part 1 (text/plain, inline)]

Here is a patch that takes a slightly different approach than my
previous one and fixes both fringe bitmaps and general image
(including "widgets": checkboxes, arrows, etc) rendering in hidpi
screens. I've attached a number of before/after screenshots.

I added two new fields `scale_x` and `scale_y` to the frame struct. I
believe going through the rif was overkilling in terms of indirections
and in terms of legibility, specially if I wanted to implement
independent scale factors for x and y in order to honour the previous
implementation. Now it's simply f->scale_x and f->scale_y. These
fields are initialized when the frame is created. They are floating
point numbers, truncation or rounding is done after scaling.

To be rigorous, these fields should belong to the terminal or to the
display_info, because they're common to all frames in the terminal
(indeed, they are closely related to resx and resy). But display_info
structs are backend specific and adding macros or inlines to frame.h
that reference them would have required including TERM_HEADER in many
many places. Also, having them at the frame level is handy since scale
factors are often, if not always, referenced in a context where a
frame is at scope.

The strategy is simple: scale fringe bitmaps and images geometries
beforehand, so as to allow geometry calculations, but postpone actual
scaling of the contents until the moment of their actual rendering to
screen. The image module has a lot of image loading functions. In
order to reduce the risk of regressions, I decided to adjust width and
height at the end of each function, immediately before returning.

Even if I've implemented this only for cairo (both with and without
gtk), adding support for x without cairo and for w32 shouldn't be
difficult at all. The ns backend simply sets both scale factors to 1,
thus disabling scaling. Other unsupported backends now should at least
carry better geometry computations, although contents won't be
actually scaled.

This doesn't support dynamic changes of dpi or scale factor. For two
or three days (and their nights) I strived to get that working. I was
able to catch xrandr resolution-change events, I was able to catch
scale-change gtk notifications, but the assumption that everything is
set just once at the beginning is firmly entrenched in the code. In
particular, rescaling fonts on the fly turned out to be an
insurmountable problem. I tried many, many things: clearing font
caches, glyph caches, matrix caches, resetting the frame font
parameter, remapping the default face (a la C-x C-+). I finally got it
working but only for the default face, I could have gone and remapped
also modeline faces, etc. I don't think such hacks are worth the
price.

In general, what people is doing to get x working with two monitors at
different dpis is to set a common gtk scale factor that is good for
the highest resolution screen (often = 2) and then scale the lowest
resolution screen down using xrandr. This is an old hack now
"officially" implemented in Ubuntu. So, if you launched  an emacs
frame in a hidpi laptop and then plugged an external monitor with the
same or lower resolution, there won't be any problem. Otherwise, if
your emacs frame started in the lowest resolution screen, then it
should be scaled up, so the only way to go is to close it and open a
frame afresh. Sorry for that.

So, all in all, this fixes hidpi problems and should also work in the
usual multi-monitor multi-dpi scenario. For a more comprehensive
solution I see no other way than writing a modern gtk backend, which
is also the way to move towards wayland.

Best regards
--
Carlos

[tetris-after.png (image/png, attachment)]

[widgets-after.png (image/png, attachment)]

[fringe-after.png (image/png, attachment)]

[tetris-before.png (image/png, attachment)]

[widgets-before.png (image/png, attachment)]

[fringe-before.png (image/png, attachment)]

[0001-Support-hidpi-screens-with-cairo.patch (text/x-patch, attachment)]

Message #8 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: 37932 <at> debbugs.gnu.org
Subject: Re: [PATCH] Support hidpi fringes and images with Cairo
Date: Fri, 25 Oct 2019 23:30:48 -0300

The patch is also online at
https://github.com/memeplex/emacs/commit/4ddc051d18d710d1af98ee5e51e9728b77d13191

Message #11 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: 37932 <at> debbugs.gnu.org
Subject: Re: [PATCH] Support hidpi fringes and images with Cairo
Date: Sat, 26 Oct 2019 03:01:47 -0300

I realized there is a problem with images. According to create-image:

Optional PROPS are additional image attributes to assign to the image,
like, e.g. `:mask MASK'.  If the property `:scale' is not given and the
display has a high resolution (more exactly, when the average width of a
character in the default font is more than 10 pixels), the image is
automatically scaled up in proportion to the default font.

So even if my patch correctly scales some images, it's scaling others
twice. Probably not all images pass through create-image.

I'll give a look at this tomorrow.

Message #14 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: 37932 <at> debbugs.gnu.org
Subject: Re: [PATCH] Support hidpi fringes and images with Cairo
Date: Sat, 26 Oct 2019 03:43:38 -0300

Ok, the problem is that widget-image-insert (in wid-edit.el) is
directly inserting the image with insert-image, without creating the
spec with create-image first, so the spec doesn't get the :scale
property. Similarly, gamegrid.el is creating its own, unscaled, image
spec.

IMO there is something smelly in using the :scale attribute like
create-image does, scaling to compensate hidpi is a very different
concern than "application level" scaling.

Anyway, do you think that:

1. I should fix those two modules to use create-image or at least add :scale, or
2. put a mark to unscaled images so that they are always scaled at the
end in x_cr_draw_image, or
3. disable this auto :scale feature when the new low-level scaling
mechanism is on

?

I think I prefer 3.

Message #17 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: 37932 <at> debbugs.gnu.org
Subject: Re: [PATCH] Support hidpi fringes and images with Cairo
Date: Sat, 26 Oct 2019 05:05:52 -0300

Here is an attempt to fix the above issues:

https://github.com/memeplex/emacs/commit/473beb25dd947c34392708ca40f1b3bf80613007

* It uses the scaling mechanism already present in image.c (notice
that imagemagick does its own scaling), so it works even without cairo
(except for fringes, of course).

* It removes the auto :scale property. I think this is the right thing
to do, applications that want to use the :scale parameter to transform
their images shouldn't lose the hidpi scaling nor explicitly call
image-compute-scaling-factor.

* The scale factor is slightly different than the one computed by
image-compute-scaling-factor, but anyway it's the scale factor that
was already computed by x/w32_scale_factor. Moreover, it has separated
x and y values.

A bit surprisingly the patch results in a code reduction of ~20LOC.
And there are image-compute-scaling-factor and image-scaling-factor
still remaining in image.el. The only user inside emacs is shr.el.

Message #20 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: 37932 <at> debbugs.gnu.org
Subject: Re: bug#37932: [PATCH] Support hidpi fringes and images with Cairo
Date: Sat, 26 Oct 2019 12:14:19 +0300

> From: Carlos Pita <carlosjosepita <at> gmail.com>
> Date: Sat, 26 Oct 2019 05:05:52 -0300
> 
> Here is an attempt to fix the above issues:
> 
> https://github.com/memeplex/emacs/commit/473beb25dd947c34392708ca40f1b3bf80613007

Thanks, but please continue posting the patches to the bug tracker, so
that the record of these discussions is complete.  It would also make
it easier for people to review the patches in Emacs while having
instant and easy access to the current code.

> * It removes the auto :scale property. I think this is the right thing
> to do, applications that want to use the :scale parameter to transform
> their images shouldn't lose the hidpi scaling nor explicitly call
> image-compute-scaling-factor.

I don't think I'm qualified to make an intelligent decision on this by
myself (and you just asked the question not long ago, and proceeded to
coding without giving anyone any time to answer it :-().  I don't know
enough about this to make such a decision.  maybe Robert, Alan, and
others would chime in and voice their opinions.  failing that, please
tell more about the pro's and con's here, so that we could see what
kind of trade-offs are we talking about.

Thanks.

Message #23 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37932 <at> debbugs.gnu.org
Subject: Re: bug#37932: [PATCH] Support hidpi fringes and images with Cairo
Date: Sat, 26 Oct 2019 06:18:12 -0300

[Message part 1 (text/plain, inline)]

On Sat, Oct 26, 2019, 6:14 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Carlos Pita <carlosjosepita <at> gmail.com>
> > Date: Sat, 26 Oct 2019 05:05:52 -0300
> >
> > Here is an attempt to fix the above issues:
> >
> >
> https://github.com/memeplex/emacs/commit/473beb25dd947c34392708ca40f1b3bf80613007
>
> Thanks, but please continue posting the patches to the bug tracker, so
>

Ok, no problem.

myself (and you just asked the question not long ago, and proceeded to
> coding without giving anyone any time to answer it :-().


It's just a proposal, I don't mind coding it again if there are good reason
to do so. I've rewritten this so many times now :)

>
>

[Message part 2 (text/html, inline)]

Message #26 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Carlos Pita <carlosjosepita <at> gmail.com>, 37932 <at> debbugs.gnu.org
Subject: Re: bug#37932: [PATCH] Support hidpi fringes and images with Cairo
Date: Sun, 27 Oct 2019 09:22:06 +0100

>>>>> On Sat, 26 Oct 2019 12:14:19 +0300, Eli Zaretskii <eliz <at> gnu.org> said:
    >> * It removes the auto :scale property. I think this is the right thing
    >> to do, applications that want to use the :scale parameter to transform
    >> their images shouldn't lose the hidpi scaling nor explicitly call
    >> image-compute-scaling-factor.

If I read the patch correctly, you now auto-scale inside image.c,
which shouldn't change anything when not using :scale. What's the
effect when code passes in eg :scale 2? Autoscaling + frame scaling,
so for a frame where scale == 2 we get * 4? I guess that would be
sensible, but weʼd need to explain it clearly. [1]

    Eli> I don't think I'm qualified to make an intelligent decision on this by
    Eli> myself (and you just asked the question not long ago, and proceeded to
    Eli> coding without giving anyone any time to answer it :-().  I don't know
    Eli> enough about this to make such a decision.  maybe Robert, Alan, and
    Eli> others would chime in and voice their opinions.  failing that, please
    Eli> tell more about the pro's and con's here, so that we could see what
    Eli> kind of trade-offs are we talking about.

I think Lars did the autoscaling work. If things continue to work as
before for the normal case, and we explain the interaction between
:scale and frame scale I have no objections.

Robert

Footnotes:
[1]  And it would get people out of the "1 physical display pixel == 1
     logical display pixel" mindset that needed to be fixed in so many
     places in the X backend.

Message #29 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Robert Pluim <rpluim <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 37932 <at> debbugs.gnu.org
Subject: Re: bug#37932: [PATCH] Support hidpi fringes and images with Cairo
Date: Sun, 27 Oct 2019 14:08:02 -0300

> If I read the patch correctly, you now auto-scale inside image.c,
> which shouldn't change anything when not using :scale. What's the

Right, AFAICS scaling of images was always done inside image.c, that's
where transformation routines are coded. The difference is more in the
usage of the :scale parameter. Previously there were two cases:

1. The caller doesn't pass a :scale in the spec. Then a :scale
property compensating for the dpi of the screen is automatically added
to the spec if the image-scaling-factor customization option is set to
'auto.  The computed scale factor assumes a representative character
to be 10px width.

2. The caller passes a :scale in the spec. Then the image is scaled
using this number and no extra dpi-adapting-scaling is done.

I see two shortcomings with this approach:

a. One I've already mentioned above: scaling required by the
application shouldn't interfere with scaling done to fit screen
resolution, which is a concern at a very different level.

b. Moreover, I'm not sure how compatible is this 10px char width
assumption with the 96dpi base resolution assumed to compute scale
factors in xterm.c and w32term.c. OTOH, the 96dpi assumption is
compatible with what gtk does. Anyway, with my patch each platform is
able to set a different base resolution if that's necessary.

> effect when code passes in eg :scale 2? Autoscaling + frame scaling,
> so for a frame where scale == 2 we get * 4? I guess that would be
> sensible, but weʼd need to explain it clearly. [1]

User code should be mostly unaware of the underlying resolution except
for very specialized cases. Currently, that is reflected in the
default 'auto image-scaling-factor. It's true that, as I commented in
point 1 above, nowadays one can pass a explicit :scale in order to
turn auto-scaling off, although in most cases that would probably
introduce a bug by *inadvertently* turning auto-scaling off. We can
add another property to the spec in order to enforce real pixel size,
or the user can divide the desired scale by the result of
image-compute-scaling-factor (in the same way the xterm backend is
circumventing gtk auto-scaling by applying the inverse scale operation
first). I prefer adding a new spec property to turn-off auto-scaling,
because that way both image-compute-scaling-factor and
image-scaling-factor could be removed from the codebase and also the
inconsistency between 96dpi and 10px criteria wouldn't be a problem
any more.

Message #32 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Robert Pluim <rpluim <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 37932 <at> debbugs.gnu.org
Subject: Re: bug#37932: [PATCH] Support hidpi fringes and images with Cairo
Date: Sun, 27 Oct 2019 14:10:04 -0300

I think I'm going to attempt scaling the fringe bitmaps in a generic
way as first suggested in #37689 [1], so as to get rid of the cairo
dependency. But, even if not a logical dependency, the organization of
that code would depend on the outcome of #37755 [2], which I had
closed in the tracker but now reopened in my mind, as shown by my last
post there :)

---

[1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37689
[2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37755

Message #35 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: Robert Pluim <rpluim <at> gmail.com>, 37932 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#37932: [PATCH] Support hidpi fringes and images with Cairo
Date: Wed, 10 Jan 2024 14:15:55 -0800

Carlos Pita <carlosjosepita <at> gmail.com> writes:

> I think I'm going to attempt scaling the fringe bitmaps in a generic
> way as first suggested in #37689 [1], so as to get rid of the cairo
> dependency. But, even if not a logical dependency, the organization of
> that code would depend on the outcome of #37755 [2], which I had
> closed in the tracker but now reopened in my mind, as shown by my last
> post there :)
>
> ---
>
> [1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37689
> [2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37755

(That was 4 years ago.)

Did you make any progress here?

Message #40 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Robert Pluim <rpluim <at> gmail.com>, 37932 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#37932: [PATCH] Support hidpi fringes and images with Cairo
Date: Thu, 11 Jan 2024 14:35:33 -0300

> Did you make any progress here?

Sorry, Stefan, there is not much that I'm able to recall about this,
it has been many years since last time I looked at the code.

AFAICR I bumped into a number of corner cases and hard assumptions
about dpi and so I concluded that the pure gtk backend was the way to
go.

Best regards,
Carlos

Message #43 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: Robert Pluim <rpluim <at> gmail.com>, 37932 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#37932: [PATCH] Support hidpi fringes and images with Cairo
Date: Thu, 11 Jan 2024 10:21:54 -0800

Carlos Pita <carlosjosepita <at> gmail.com> writes:

> Sorry, Stefan, there is not much that I'm able to recall about this,
> it has been many years since last time I looked at the code.
>
> AFAICR I bumped into a number of corner cases and hard assumptions
> about dpi and so I concluded that the pure gtk backend was the way to
> go.

Thanks for getting back to us.  Do you think some version of your
changes would still be worth installing, or would they need
more work to be considered an improvement?

Could you send your latest changes as a patch?

Message #46 received at 37932 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Robert Pluim <rpluim <at> gmail.com>, 37932 <at> debbugs.gnu.org,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#37932: [PATCH] Support hidpi fringes and images with Cairo
Date: Thu, 11 Jan 2024 15:38:08 -0300

I would say that the lengthy discussions in:

- https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37689
- https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37770
- https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37755

are my only legacy. Rereading them I found some relevant remarks about
the problems I bumped into at the time.

But that's it. My apologies, I'm checking and I'm not even able to
find the fork where I had been working on this.

Why are you interested in it? Isn't it kind of obsolete now that there
is a pure GTK backend?



On Thu, Jan 11, 2024 at 3:21 PM Stefan Kangas <stefankangas <at> gmail.com> wrote:
>
> Carlos Pita <carlosjosepita <at> gmail.com> writes:
>
> > Sorry, Stefan, there is not much that I'm able to recall about this,
> > it has been many years since last time I looked at the code.
> >
> > AFAICR I bumped into a number of corner cases and hard assumptions
> > about dpi and so I concluded that the pure gtk backend was the way to
> > go.
>
> Thanks for getting back to us.  Do you think some version of your
> changes would still be worth installing, or would they need
> more work to be considered an improvement?
>
> Could you send your latest changes as a patch?

Message #51 received at 37932-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: Robert Pluim <rpluim <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>,
 37932-done <at> debbugs.gnu.org
Subject: Re: bug#37932: [PATCH] Support hidpi fringes and images with Cairo
Date: Thu, 11 Jan 2024 11:31:12 -0800

Carlos Pita <carlosjosepita <at> gmail.com> writes:

> I would say that the lengthy discussions in:
>
> - https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37689
> - https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37770
> - https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37755
>
> are my only legacy. Rereading them I found some relevant remarks about
> the problems I bumped into at the time.
>
> But that's it. My apologies, I'm checking and I'm not even able to
> find the fork where I had been working on this.

OK, thanks for looking nevertheless.

> Why are you interested in it? Isn't it kind of obsolete now that there
> is a pure GTK backend?

It hasn't yet been obsoleted in Emacs.  Maybe eventually, but not yet.
Until that happens, we're always happy to take improvements.

I'm closing this bug now.  Thanks for your time and effort.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.