GNU bug report logs -
#40845
SVG rendering issues
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 40845 in the body.
You can then email your comments to 40845 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 12:20:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Clément Pit-Claudel <cpitclaudel <at> gmail.com>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Sat, 25 Apr 2020 12:20:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Hi all,
As discussed on the mailing list, a number of issues currently exist with our SVG rendering implementation. I have tried to summarize the ones I'm aware of in the following example.
(with-current-buffer (get-buffer-create "*svg bugs*")
(erase-buffer)
(require 'face-remap)
(setq text-scale-mode-amount 10)
(text-scale-mode)
(let ((svg (svg-create 16 16)))
(svg-ellipse svg 8 8 4 4)
(insert "Text: ")
(print (svg-image svg :ascent 100))
(insert-image (svg-image svg :ascent 100))
(insert-image (svg-image svg :scale 5.0 :ascent 'center :foreground "red" :background "darkgreen"))
(add-text-properties
(point-min) (point-max)
'(face (:foreground "orange" :background "purple")
mouse-face '(:foreground "purple" :background "orange"))))
(pop-to-buffer (current-buffer)))
The issues:
1. Manually scaling an image, as is done for the second image, doesn't re-render the svg: is scales the bitmap-rendered version of it, causing blurriness.
2. The SVG images don't inherit the background of the current face; instead, they inherit the background of the default face.
3. The SVG images don't inherit the foreground of the current face; instead, they use a black foreground.
4. The :foreground keyword has no effect on svg images.
5. The images are not scaled with the text: changing text-scale-mode-amount doesn't change the size of the images.
For 1, 2, 3, and 4, the expected behavior is easy to define:
- For 1, the image should be scaled before being rasterized.
- For 2 and 3, the image should inherit the characteristics of the current face, and be re-rendered if that face changes (e.g. when mouse-face applies, which is important for buttons)
- For 4, the :foreground property should be respected
For 5, it's a bit trickier. One option would be to accept a float-valued :height specification and treat that as a multiple of the current font size.
A partial workaround for 2 is to add an explicit :background, but it doesn't help with face changes, such as applying a mouse-face. For 1 and 5 it can be enough to set the size in the SVG and add hooks, but that only works easily for SVGs created within emacs. For 3 and 4, I don't know of a workaround except modifying the SVG.
Cheers,
Clément.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 14:35:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 40845 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Sat, Apr 25, 2020 at 12:20 PM Clément Pit-Claudel
<cpitclaudel <at> gmail.com> wrote:
>
> Hi all,
>
> As discussed on the mailing list, a number of issues currently exist with our SVG rendering implementation. I have tried to summarize the ones I'm aware of in the following example.
>
> (with-current-buffer (get-buffer-create "*svg bugs*")
> (erase-buffer)
> (require 'face-remap)
> (setq text-scale-mode-amount 10)
> (text-scale-mode)
> (let ((svg (svg-create 16 16)))
> (svg-ellipse svg 8 8 4 4)
> (insert "Text: ")
> (print (svg-image svg :ascent 100))
> (insert-image (svg-image svg :ascent 100))
> (insert-image (svg-image svg :scale 5.0 :ascent 'center :foreground "red" :background "darkgreen"))
> (add-text-properties
> (point-min) (point-max)
> '(face (:foreground "orange" :background "purple")
> mouse-face '(:foreground "purple" :background "orange"))))
> (pop-to-buffer (current-buffer)))
>
> The issues:
>
> 1. Manually scaling an image, as is done for the second image, doesn't re-render the svg: is scales the bitmap-rendered version of it, causing blurriness.
> 2. The SVG images don't inherit the background of the current face; instead, they inherit the background of the default face.
> 3. The SVG images don't inherit the foreground of the current face; instead, they use a black foreground.
> 4. The :foreground keyword has no effect on svg images.
> 5. The images are not scaled with the text: changing text-scale-mode-amount doesn't change the size of the images.
I would like to add
6. When the cursor is over an SVG image, it is displayed with a box
around it rather than with inverted video as characters are.
I've played around a while ago in an attempt to fix these issues, and
be able to define "character-like" SVG glyphs. My approach was to add
a display spec of the form (gen FN), which calls FN with the current
face parameters as arguments whenever redisplaying the spec, then uses
the return value (an image spec) as the actual spec to be displayed.
It's probably quite slow, but (as of June last) it works, even when
displaying the same buffer twice. Calling elisp from the redisplay
engine is, of course, something we don't really want to have more of,
but we'd do so anyway for a `when' spec (in fact, an alternative
approach to get this working with old Emacsen was to abuse a `when'
spec to set the image spec correctly).
The attached patch, IIRC, is the main part. As you can see, it leaves
most of the work to the Lisp side of things. It can fix all issues
mentioned above by modifying the SVG as required.
Also, IIRC, 3 is an rsvg issue.
[gen.diff (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 15:31:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> From: Pip Cet <pipcet <at> gmail.com>
> Date: Sat, 25 Apr 2020 14:34:11 +0000
> Cc: 40845 <at> debbugs.gnu.org
>
> 6. When the cursor is over an SVG image, it is displayed with a box
> around it rather than with inverted video as characters are.
>
> I've played around a while ago in an attempt to fix these issues, and
> be able to define "character-like" SVG glyphs. My approach was to add
> a display spec of the form (gen FN), which calls FN with the current
> face parameters as arguments whenever redisplaying the spec, then uses
> the return value (an image spec) as the actual spec to be displayed.
I don't think I understand why we need to call a function for this.
This is about displaying an image with a specific background color,
isn't it?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 15:47:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> From: Pip Cet <pipcet <at> gmail.com>
> Date: Sat, 25 Apr 2020 14:34:11 +0000
> Cc: 40845 <at> debbugs.gnu.org
>
> > 3. The SVG images don't inherit the foreground of the current face; instead, they use a black foreground.
>
> Also, IIRC, 3 is an rsvg issue.
I don't think I understand what item 3 is complaining about. If I
visit etc/images/splash.svg, it gets the background color of the
default face. So is the complaint that it takes the background from
the default face instead of the current face? That should be easy to
fix, I think, as this code in svg_load_image shows:
Emacs_Color background;
Lisp_Object specified_bg = image_spec_value (img->spec, QCbackground, NULL);
if (!STRINGP (specified_bg)
|| !FRAME_TERMINAL (f)->defined_color_hook (f,
SSDATA (specified_bg),
&background,
false,
false))
FRAME_TERMINAL (f)->query_frame_background_color (f, &background);
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 15:50:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sat, Apr 25, 2020 at 3:30 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > I've played around a while ago in an attempt to fix these issues, and
> > be able to define "character-like" SVG glyphs. My approach was to add
> > a display spec of the form (gen FN), which calls FN with the current
> > face parameters as arguments whenever redisplaying the spec, then uses
> > the return value (an image spec) as the actual spec to be displayed.
>
> I don't think I understand why we need to call a function for this.
> This is about displaying an image with a specific background color,
> isn't it?
Even if the problem were just the choice of background color, that
would require significant and non-trivial changes for some cases: for
example, an emoji might have to choose foreground colors that provide
sufficient contrast to the background color, and antialiasing and
sub-pixel rendering would also need to be taken into account.
But, at least for me, it's not: I want to be able to define something
that behaves like a character, including displaying differently in
different frames and depending on different face parameters such as
weight, slant, and RTL-ness. I don't really see a way of doing that
satisfactorily without invoking user code outside of the display
engine/image backend code.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 16:12:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> From: Pip Cet <pipcet <at> gmail.com>
> Date: Sat, 25 Apr 2020 15:48:39 +0000
> Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org
>
> On Sat, Apr 25, 2020 at 3:30 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
> >
> > I don't think I understand why we need to call a function for this.
> > This is about displaying an image with a specific background color,
> > isn't it?
>
> Even if the problem were just the choice of background color, that
> would require significant and non-trivial changes for some cases: for
> example, an emoji might have to choose foreground colors that provide
> sufficient contrast to the background color, and antialiasing and
> sub-pixel rendering would also need to be taken into account.
We are talking about displaying images, not characters, right? So why
are emoji relevant to this?
> I want to be able to define something that behaves like a character,
> including displaying differently in different frames and depending
> on different face parameters such as weight, slant, and RTL-ness.
All of that is possible with images, so I don't think I understand why
we need to handle this as a character. This very bug report was filed
because I said we shouldn't use characters and fonts for these
purposes, so let's not discuss that alternative, please, at least not
here.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 16:43:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On 25/04/2020 11.46, Eli Zaretskii wrote:
>> From: Pip Cet <pipcet <at> gmail.com>
>> Date: Sat, 25 Apr 2020 14:34:11 +0000
>> Cc: 40845 <at> debbugs.gnu.org
>>
>>> 3. The SVG images don't inherit the foreground of the current face; instead, they use a black foreground.
>>
>> Also, IIRC, 3 is an rsvg issue.
>
> I don't think I understand what item 3 is complaining about. If I
> visit etc/images/splash.svg, it gets the background color of the
> default face. So is the complaint that it takes the background from
> the default face instead of the current face? That should be easy to
> fix, I think, as this code in svg_load_image shows:
That's item 2:
2. The SVG images don't inherit the background of the current face; instead, they inherit the background of the default face.
and you summarized it exactly right. Item 3 is about the color of the circle that the repro draws. . It should be purple, but it's black.
Clément.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 17:04:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> Cc: 40845 <at> debbugs.gnu.org
> From: Clément Pit-Claudel <cpitclaudel <at> gmail.com>
> Date: Sat, 25 Apr 2020 12:42:07 -0400
>
> >>> 3. The SVG images don't inherit the foreground of the current face; instead, they use a black foreground.
> >>
> >> Also, IIRC, 3 is an rsvg issue.
> >
> > I don't think I understand what item 3 is complaining about. If I
> > visit etc/images/splash.svg, it gets the background color of the
> > default face. So is the complaint that it takes the background from
> > the default face instead of the current face? That should be easy to
> > fix, I think, as this code in svg_load_image shows:
>
> That's item 2:
>
> 2. The SVG images don't inherit the background of the current face; instead, they inherit the background of the default face.
>
> and you summarized it exactly right. Item 3 is about the color of the circle that the repro draws. . It should be purple, but it's black.
Ah, sorry.
But IMO the foreground of an image shouldn't be affected by the
current face's foreground. Images should come with their own colors,
and be displayed like that. I think in the context of the discussion
that led to this bug report it's actually almost a must: we want
images with attractive colors to serve as the icons, we don't want
them to be affected by whatever face happens to be nearby.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 17:25:01 GMT)
Full text and
rfc822 format available.
Message #29 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On 25/04/2020 13.02, Eli Zaretskii wrote:
> IMO the foreground of an image shouldn't be affected by the
> current face's foreground. Images should come with their own colors,
> and be displayed like that. I think in the context of the discussion
> that led to this bug report it's actually almost a must: we want
> images with attractive colors to serve as the icons, we don't want
> them to be affected by whatever face happens to be nearby.
Yes, of course: if an image has a foreground color, it should be respected.
But it's not uncommon for SVG images to not include a foreground color, as shown in the repro. In that case, the image should use the current foreground color, I think.
(of course, a :foreground keyword, if any, should take precedence; that is issue 4).
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 17:40:02 GMT)
Full text and
rfc822 format available.
Message #32 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sat, Apr 25, 2020 at 4:10 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > From: Pip Cet <pipcet <at> gmail.com>
> > Date: Sat, 25 Apr 2020 15:48:39 +0000
> > Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org
> >
> > On Sat, Apr 25, 2020 at 3:30 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > >
> > > I don't think I understand why we need to call a function for this.
> > > This is about displaying an image with a specific background color,
> > > isn't it?
> >
> > Even if the problem were just the choice of background color, that
> > would require significant and non-trivial changes for some cases: for
> > example, an emoji might have to choose foreground colors that provide
> > sufficient contrast to the background color, and antialiasing and
> > sub-pixel rendering would also need to be taken into account.
>
> We are talking about displaying images, not characters, right?
Correct: images which behave like character glyphs, but don't actually
have character codepoints, and aren't implemented by fonts.
> So why are emoji relevant to this?
An emoji is an example of an image which needs to know face properties
to blend in, or stick out, or whatever it is people who use them want
to happen. The intention was not that you'd use an emoji codepoint and
a font providing a character for that codepoint, which is a silly way
of implementing emoji.
> > I want to be able to define something that behaves like a character,
> > including displaying differently in different frames and depending
> > on different face parameters such as weight, slant, and RTL-ness.
>
> All of that is possible with images, so I don't think I understand why
> we need to handle this as a character.
Yes, my approach uses images, and no, it doesn't "handle this as a
character". It makes the glyph's apparent behavior match that of
character glyphs, while actually displaying an image spec which
changes with the properties of surrounding text, etc.
> This very bug report was filed
> because I said we shouldn't use characters and fonts for these
> purposes, so let's not discuss that alternative, please, at least not
> here.
No one was.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 17:47:01 GMT)
Full text and
rfc822 format available.
Message #35 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sat, Apr 25, 2020 at 01:24:13PM -0400, Clément Pit-Claudel wrote:
> On 25/04/2020 13.02, Eli Zaretskii wrote:
> > IMO the foreground of an image shouldn't be affected by the
> > current face's foreground. Images should come with their own colors,
> > and be displayed like that. I think in the context of the discussion
> > that led to this bug report it's actually almost a must: we want
> > images with attractive colors to serve as the icons, we don't want
> > them to be affected by whatever face happens to be nearby.
>
> Yes, of course: if an image has a foreground color, it should be
> respected. But it's not uncommon for SVG images to not include a
> foreground color, as shown in the repro. In that case, the image
> should use the current foreground color, I think. (of course, a
> :foreground keyword, if any, should take precedence; that is issue
> 4).
Lars fixed the foreground issue for eww by wrapping svg files in
another svg. See svg--wrap-svg in shr.el (bug#37159).
I think this is the only practical way to handle svg files with no
foreground colour set. To do this when loading _any_ svg we’d probably
have to move it into create-image or image.c.
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 18:08:02 GMT)
Full text and
rfc822 format available.
Message #38 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> From: Pip Cet <pipcet <at> gmail.com>
> Date: Sat, 25 Apr 2020 17:38:31 +0000
> Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org
>
> Correct: images which behave like character glyphs, but don't actually
> have character codepoints, and aren't implemented by fonts.
>
> > So why are emoji relevant to this?
>
> An emoji is an example of an image which needs to know face properties
> to blend in, or stick out, or whatever it is people who use them want
> to happen. The intention was not that you'd use an emoji codepoint and
> a font providing a character for that codepoint, which is a silly way
> of implementing emoji.
>
> > > I want to be able to define something that behaves like a character,
> > > including displaying differently in different frames and depending
> > > on different face parameters such as weight, slant, and RTL-ness.
> >
> > All of that is possible with images, so I don't think I understand why
> > we need to handle this as a character.
>
> Yes, my approach uses images, and no, it doesn't "handle this as a
> character". It makes the glyph's apparent behavior match that of
> character glyphs, while actually displaying an image spec which
> changes with the properties of surrounding text, etc.
>
> > This very bug report was filed
> > because I said we shouldn't use characters and fonts for these
> > purposes, so let's not discuss that alternative, please, at least not
> > here.
>
> No one was.
Then I guess I don't understand your implementation at all.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 18:09:01 GMT)
Full text and
rfc822 format available.
Message #41 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sat, Apr 25, 2020 at 5:46 PM Alan Third <alan <at> idiocy.org> wrote:
> On Sat, Apr 25, 2020 at 01:24:13PM -0400, Clément Pit-Claudel wrote:
> > Yes, of course: if an image has a foreground color, it should be
> > respected. But it's not uncommon for SVG images to not include a
> > foreground color, as shown in the repro. In that case, the image
> > should use the current foreground color, I think. (of course, a
> > :foreground keyword, if any, should take precedence; that is issue
> > 4).
For reference, the relevant chunk of librsvg source code is this:
// https://www.w3.org/TR/SVG/color.html#ColorProperty
make_property!(
ComputedValues,
Color,
// The SVG spec allows the user agent to choose its own default
for the "color" property.
// We don't allow passing in an initial CSS in the public API, so
we'll start with black.
//
// See https://bugzilla.gnome.org/show_bug.cgi?id=764808 for a
case where this would
// be useful - rendering equations with currentColor, so they take
on the color of the
// surrounding text.
default: cssparser::RGBA::new(0, 0, 0, 0xff),
inherits_automatically: true,
newtype_parse: cssparser::RGBA,
);
> Lars fixed the foreground issue for eww by wrapping svg files in
> another svg. See svg--wrap-svg in shr.el (bug#37159).
>
> I think this is the only practical way to handle svg files with no
> foreground colour set. To do this when loading _any_ svg we’d probably
> have to move it into create-image or image.c.
I think it's a neat hack, but it's just one of the more obvious ways
of working around an annoying limitation of librsvg (Ideally, we'd use
a library without such a limitation by default and fall back to
modifying/wrapping SVGs when using a limited library.)
For applications which change glyphs' foreground colors a lot, it
might be preferrable to coax rsvg into providing an alpha-only map for
the foreground plus an RGBA map for the background and blend them
together, and that approach would work for other image types.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 19:43:01 GMT)
Full text and
rfc822 format available.
Message #44 received at 40845 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Sat, Apr 25, 2020 at 6:07 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
or these
> Then I guess I don't understand your implementation at all.
My use case is to include a glyph which is supposed to look like a
character, but doesn't actually have a unicode codepoint. (I'm sorry
if this differs from the use cases you're exclusively concerned with,
but it appeared to be relevant enough to Clément's problem that I
assumed he was having a similar issue).
That means that we want to use an image spec, not a character in a
font; but that image spec depends on face/font properties, because we
want to blend in with surrounding text. The most obvious ones are
foreground and background color and size, but slant and weight would
also affect properly-rendered character-like images.
It seems fairly obvious to me that it's a bad idea to do all the work
in the display engine or in C code: sub-pixel rendering and
anti-aliasing are hard to get right. A character-like glyph might need
a third color which provides sufficient contrast to the foreground and
background colors, and that color space calculation is complicated
enough to be moved to Lisp.
So I moved it to Lisp: there's Lisp code which is passed the font/face
properties, and returns an image spec that's appropriate for that.
The attached patch actually applies to and works with master, and
includes an example. It can no longer easily be demonstrated to do the
right thing when the same buffer is displayed in different frames,
because Emacs currently applies text scaling per buffer rather than
per frame (IMHO, that's a bug). It also doesn't properly display the
cursor as a block cursor when it's over the glyph, because I can no
longer find the code which allowed me to tell, from Lisp, whether that
was the case.
And of course it doesn't use the right font metrics, because these
currently aren't exposed to Lisp.
But all these limitations are fixable.
[0001-support-generated-display-specs.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 25 Apr 2020 20:12:02 GMT)
Full text and
rfc822 format available.
Message #47 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> From: Pip Cet <pipcet <at> gmail.com>
> Date: Sat, 25 Apr 2020 19:41:31 +0000
> Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org
>
> My use case is to include a glyph which is supposed to look like a
> character, but doesn't actually have a unicode codepoint. (I'm sorry
> if this differs from the use cases you're exclusively concerned with,
> but it appeared to be relevant enough to Clément's problem that I
> assumed he was having a similar issue).
>
> That means that we want to use an image spec, not a character in a
> font; but that image spec depends on face/font properties, because we
> want to blend in with surrounding text. The most obvious ones are
> foreground and background color and size, but slant and weight would
> also affect properly-rendered character-like images.
If we want to display an image, the logical way would be to use an
image spec, which is already supported, enhancing it as needed. I
don't see anything in the above attributes that would be hard to have
within the existing framework of how we display images. I don't
really understand how you can make images "slant" or "bold" (unless
they were like that to begin with), or why would we want to, but maybe
there's some way of interpreting that as well.
None of this requires a new spec or calling Lisp. All of the metrics
of the current face's font are known by the display code, and there
are many that cannot be obtained from Lisp, like the current line
height. Doing these calculations in Lisp is IMO the worst possible
way of using Lisp callouts from the display engine: this has all the
disadvantages of calling Lisp, and none of the advantages.
> It seems fairly obvious to me that it's a bad idea to do all the work
> in the display engine or in C code: sub-pixel rendering and
> anti-aliasing are hard to get right.
Aren't we talking about displaying existing images, which someone
already prepared while using all those techniques? Why would the
display engine want or need to modify the image using them?
> A character-like glyph might need a third color which provides
> sufficient contrast to the foreground and background colors, and
> that color space calculation is complicated enough to be moved to
> Lisp.
Within the context of what these icons are supposed to be used for, I
envision the images coming with bright, catchy colors that should be
left alone, not enhanced by color and contrast tweaking of whatever
happens to be the face colors the user wants. Those who design such
icons do a much better job than any Lisp program.
> But all these limitations are fixable.
They wouldn't have existed if you used the code which displays images.
So I still don't understand the motivation for this approach. (Read:
I think it's wrong.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 26 Apr 2020 10:17:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sat, Apr 25, 2020 at 8:11 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > From: Pip Cet <pipcet <at> gmail.com>
> > Date: Sat, 25 Apr 2020 19:41:31 +0000
> > Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org
> >
> > My use case is to include a glyph which is supposed to look like a
> > character, but doesn't actually have a unicode codepoint.
> >
> > That means that we want to use an image spec, not a character in a
> > font; but that image spec depends on face/font properties, because we
> > want to blend in with surrounding text. The most obvious ones are
> > foreground and background color and size, but slant and weight would
> > also affect properly-rendered character-like images.
>
> If we want to display an image, the logical way would be to use an
> image spec, which is already supported, enhancing it as needed.
I want to display a glyph that looks different depending on its
textual context. That means it's not "an image", in the traditional
sense. and enhancing image specs to handle such context-dependent
glyphs is wrong, because it moves code into the image backend that
applies to all image backends.
To be clear, to me "an image" is a compressed way of storing RGBA
information for each pixel in a rectangle. It has no concept of
foreground color or background color or slant or weight.
The enhancement I propose is a layer of indirection, allowing the
image spec itself to be adjusted to face/font properties. Whether or
not it's best to use a Lisp function for that, or a hash table, is
something I'm not clear about yet.
> I don't see anything in the above attributes that would be hard to have
> within the existing framework of how we display images.
I don't see anything in the above attributes that would be possible to
have without major changes to image backends, for features that do not
depend on the image backend.
> I don't
> really understand how you can make images "slant" or "bold" (unless
> they were like that to begin with), or why would we want to, but maybe
> there's some way of interpreting that as well.
Because we want the glyph to blend in with surrounding text. Like a
character. Thus "character-like glyph". It's not an image. My use case
is displaying character-like glyphs, implemented as
dynamically-generated images, not to display final images.
And, no, there's no way for an image backend to interpret that. We
have to use different image specs.
> None of this requires a new spec or calling Lisp.
I don't see how to do it without a new spec. I do see an obvious way
of doing it without calling Lisp, by using, say, a hash table. That's
an implementation detail, and one I'm willing to argue about, but "you
want images, not character-like glyphs" isn't an argument, it's simply
a statement that you don't want my use case to be supported.
> All of the metrics
> of the current face's font are known by the display code, and there
> are many that cannot be obtained from Lisp, like the current line
> height.
Indeed, so the display code has to expose those to Lisp.
> Doing these calculations in Lisp is IMO the worst possible
> way of using Lisp callouts from the display engine: this has all the
> disadvantages of calling Lisp, and none of the advantages.
The advantage is my use case is supported, and with your proposal
(which I don't understand completely, but it appears to be a
perfunctory implementation of "foreground"/"background" colors for
images) it isn't.
Whether we should call Lisp (which would, in practice, return a cached
image spec most of the time) or use a hash table (and we'd have to
somehow trigger Lisp updating that hash table) is something I'm not
clear about. Since we're already calling Lisp from code in the
vicinity, it seems relatively safe to do so.
> > It seems fairly obvious to me that it's a bad idea to do all the work
> > in the display engine or in C code: sub-pixel rendering and
> > anti-aliasing are hard to get right.
>
> Aren't we talking about displaying existing images, which someone
> already prepared while using all those techniques?
I'm talking about displaying character-like glyphs.
> Why would the
> display engine want or need to modify the image using them?
To make the glyph character-like.
> > A character-like glyph might need a third color which provides
> > sufficient contrast to the foreground and background colors, and
> > that color space calculation is complicated enough to be moved to
> > Lisp.
>
> Within the context of what these icons are supposed to be used for, I
> envision the images coming with bright, catchy colors that should be
> left alone, not enhanced by color and contrast tweaking of whatever
> happens to be the face colors the user wants.
I'm not sure which icons you're thinking about, but I certainly don't
want my symbols to have "bright, catchy colors that should be left
alone". That's one valid thing for the Lisp code to do, to leave the
image alone and not modify it, but I want the option to have
character-like glyphs, which look like characters but aren't, and that
means they don't have bright, catchy colors that should be left alone.
> Those who design such
> icons do a much better job than any Lisp program.
That's an end user decision, not something we should assume a priori.
> > But all these limitations are fixable.
> They wouldn't have existed if you used the code which displays images.
I want to display character-like glyphs. I don't see a way doing that
with existing image specs, because the image specs do not depend on
face/font properties. I'm still not sure what you're proposing, but
making images depend in a brutish way on "foreground" and "background"
color is a highly specific and limited solution that doesn't solve my
use case at all.
> So I still don't understand the motivation for this approach. (Read:
> I think it's wrong.)
I'm still not sure whether you're suggesting anything that would
handle my use case even remotely, or simply saying it's not a use case
that Emacs should be extensible to. I've certainly not seen any
suggestion that would do the former.
But, just to be clear, I posted this code because I'd already made the
minor effort to get a new (and, IMHO, important) Emacs feature to
work. I deliberately decided not to submit it, because while it is a
new, generally useful feature Emacs sorely lacks, there are both
productive discussions to have about it (such as whether it uses a
hash table or a Lisp function calls) and unproductive ones, and the
latter in particular translate to a major effort that I did not at the
time feel was worth it.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 26 Apr 2020 14:39:01 GMT)
Full text and
rfc822 format available.
Message #53 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> From: Pip Cet <pipcet <at> gmail.com>
> Date: Sun, 26 Apr 2020 10:15:39 +0000
> Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org
>
> > If we want to display an image, the logical way would be to use an
> > image spec, which is already supported, enhancing it as needed.
>
> I want to display a glyph that looks different depending on its
> textual context. That means it's not "an image", in the traditional
> sense.
Then I guess we are talking about two different use cases. This bug
report was filed in the context of this discussion:
https://lists.gnu.org/archive/html/emacs-devel/2020-04/msg01386.html
where I said that I think such UI effects should be implemented by
using images, not special characters and special fonts.
> and enhancing image specs to handle such context-dependent
> glyphs is wrong, because it moves code into the image backend that
> applies to all image backends.
"Image backend" in this context is the image libraries, like librsvg
and libpng, is that right? If so, which one of them can do the stuff
Clément reported missing?
> The enhancement I propose is a layer of indirection, allowing the
> image spec itself to be adjusted to face/font properties.
The kind of adjustments that we need for the use case which prompted
this discussion can be done inside the display engine. Or at least I
don't see a reason why it couldn't be.
> > I don't
> > really understand how you can make images "slant" or "bold" (unless
> > they were like that to begin with), or why would we want to, but maybe
> > there's some way of interpreting that as well.
>
> Because we want the glyph to blend in with surrounding text. Like a
> character.
Not really, not in the use case discussed here.
> And, no, there's no way for an image backend to interpret that. We
> have to use different image specs.
A Lisp program that prepares such display can prepare the spec
accordingly, if needed. Any effects that cannot be explicitly
produced by Lisp and we'd need the display code to produce can be
communicated using the image spec keywords, like we always do. For
example, if we want the image to scale with the surrounding face, we
can have a special value of the :scale attribute. And similarly with
other attributes; we can also invent new keywords for attributes
currently not supported.
> "you want images, not character-like glyphs" isn't an argument, it's
> simply a statement that you don't want my use case to be supported.
It just means your use case is different from what is being discussed
here, and serves purposes other than those which prompted this bug
report. I don't think we should mix them.
> I'm talking about displaying character-like glyphs.
And I'm not. This bug report is about a different use case.
> > Within the context of what these icons are supposed to be used for, I
> > envision the images coming with bright, catchy colors that should be
> > left alone, not enhanced by color and contrast tweaking of whatever
> > happens to be the face colors the user wants.
>
> I'm not sure which icons you're thinking about, but I certainly don't
> want my symbols to have "bright, catchy colors that should be left
> alone".
I suggest to read the above discussion, and also look at the UI
look-and-feel that is being sought. You will see many examples there
of the kind of images that people would like to see. I think their
vivid colors is one of the main aspects that attract users to such UI.
> I'm still not sure whether you're suggesting anything that would
> handle my use case even remotely, or simply saying it's not a use case
> that Emacs should be extensible to.
I guess I'm saying the latter. Maybe if you explained when such use
cases arise, I will agree with you. But in any case, it's a different
use case.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 26 Apr 2020 19:02:01 GMT)
Full text and
rfc822 format available.
Message #56 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sun, Apr 26, 2020 at 2:38 PM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > From: Pip Cet <pipcet <at> gmail.com>
> > Date: Sun, 26 Apr 2020 10:15:39 +0000
> > Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org
> >
> > > If we want to display an image, the logical way would be to use an
> > > image spec, which is already supported, enhancing it as needed.
> >
> > I want to display a glyph that looks different depending on its
> > textual context. That means it's not "an image", in the traditional
> > sense.
>
> Then I guess we are talking about two different use cases.
A more general one, and a more limited and specific one. As it
happens, the fix for the more general one is simpler.
> This bug
> report was filed in the context of this discussion:
>
> https://lists.gnu.org/archive/html/emacs-devel/2020-04/msg01386.html
>
> where I said that I think such UI effects should be implemented by
> using images, not special characters and special fonts.
Indeed. You're right about that. We should use dynamically-generated
image specs for them. That's what my patch allows for.
> > and enhancing image specs to handle such context-dependent
> > glyphs is wrong, because it moves code into the image backend that
> > applies to all image backends.
>
> "Image backend" in this context is the image libraries, like librsvg
> and libpng, is that right?
Yes.
> If so, which one of them can do the stuff
> Clément reported missing?
Err, who said they could? I'm saying they _shouldn't_. It's not the
image backend's job to alter and adapt an image, just to read a
compressed version of a bitmap and uncompress it, interpreting
whatever method of compression (say, vectorization) was chosen.
> > The enhancement I propose is a layer of indirection, allowing the
> > image spec itself to be adjusted to face/font properties.
>
> The kind of adjustments that we need for the use case which prompted
> this discussion can be done inside the display engine.
As opposed to doing them when the spec is evaluated?
> > And, no, there's no way for an image backend to interpret that. We
> > have to use different image specs.
>
> A Lisp program that prepares such display can prepare the spec
> accordingly, if needed.
That's what I'm proposing: call a Lisp program to prepare a spec
whenever we display the glyph. Not more often, which is wasteful.
Except you're currently wrong: a Lisp program cannot prepare "the
spec" accordingly, because there might be several required specs in
effect at the same time, for example.
> Any effects that cannot be explicitly
> produced by Lisp and we'd need the display code to produce can be
> communicated using the image spec keywords, like we always do. For
> example, if we want the image to scale with the surrounding face, we
> can have a special value of the :scale attribute. And similarly with
> other attributes; we can also invent new keywords for attributes
> currently not supported.
Yes, that describes what I'm doing, except for the important point
that the spec is generated per occurence of the glyph rather than once
globally.
> > "you want images, not character-like glyphs" isn't an argument, it's
> > simply a statement that you don't want my use case to be supported.
>
> It just means your use case is different from what is being discussed
> here, and serves purposes other than those which prompted this bug
> report. I don't think we should mix them.
It's strict containment: my use case is more general.
> > > Within the context of what these icons are supposed to be used for, I
> > > envision the images coming with bright, catchy colors that should be
> > > left alone, not enhanced by color and contrast tweaking of whatever
> > > happens to be the face colors the user wants.
> >
> > I'm not sure which icons you're thinking about, but I certainly don't
> > want my symbols to have "bright, catchy colors that should be left
> > alone".
>
> I suggest to read the above discussion, and also look at the UI
> look-and-feel that is being sought. You will see many examples there
> of the kind of images that people would like to see. I think their
> vivid colors is one of the main aspects that attract users to such UI.
I did read the discussion, and the main thing mentioned about their
colors is that they shouldn't be fixed, i.e. not "left alone".
> > I'm still not sure whether you're suggesting anything that would
> > handle my use case even remotely, or simply saying it's not a use case
> > that Emacs should be extensible to.
>
> I guess I'm saying the latter.
My impression is you're treating extensibility itself as something to
be avoided, not as something on the benefit side of a cost-benefit
analysis (cost: 5 lines of actual code. benefit: solves both the more
specific use case sought here and the more general use case I need).
> Maybe if you explained when such use
> cases arise, I will agree with you. But in any case, it's a different
> use case.
A more general one.
As to how they arise: mathematicians, for example, have a
long-standing tradition of making up symbols. Those new symbols aren't
(and shouldn't be) in Unicode, and there are many other symbols that,
for example, cannot be in Unicode for legal or ethical reasons. Users
should be able to display such symbols in an Emacs buffer and have
them be as close to character glyphs (or different from them) as they
are willing to make them.
I remain convinced that I'm not smart enough to figure out in advance
the precise set of use cases I want a feature for. You shouldn't limit
generality to handle only those cases you're convinced are useful.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 26 Apr 2020 21:18:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sat, Apr 25, 2020 at 06:46:51PM +0100, Alan Third wrote:
> On Sat, Apr 25, 2020 at 01:24:13PM -0400, Clément Pit-Claudel wrote:
> > On 25/04/2020 13.02, Eli Zaretskii wrote:
> > > IMO the foreground of an image shouldn't be affected by the
> > > current face's foreground. Images should come with their own colors,
> > > and be displayed like that. I think in the context of the discussion
> > > that led to this bug report it's actually almost a must: we want
> > > images with attractive colors to serve as the icons, we don't want
> > > them to be affected by whatever face happens to be nearby.
> >
> > Yes, of course: if an image has a foreground color, it should be
> > respected. But it's not uncommon for SVG images to not include a
> > foreground color, as shown in the repro. In that case, the image
> > should use the current foreground color, I think. (of course, a
> > :foreground keyword, if any, should take precedence; that is issue
> > 4).
>
> Lars fixed the foreground issue for eww by wrapping svg files in
> another svg. See svg--wrap-svg in shr.el (bug#37159).
>
> I think this is the only practical way to handle svg files with no
> foreground colour set. To do this when loading _any_ svg we’d probably
> have to move it into create-image or image.c.
FWIW, with some experimentation I’ve found that this wrapping method
can easily sort the scaling problems too.
The process would have to go, roughly:
* load the svg code
* parse it with librsvg
* extract the size
* calculate the new size (compute_image_size in image.c should work
fine)
* base64 encode the original svg
* wrap it in the new svg code
* parse
* produce the bitmap
Then we’re probably as well skipping all the matrix calculation stuff,
the same as we do for ImageMagick, as the bitmap should be the exact
size we want.
I’m happy to give this a go, but I’m not sure about base64 encoding.
Do we already have a way of doing that to a char buffer?
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 26 Apr 2020 22:49:01 GMT)
Full text and
rfc822 format available.
Message #62 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On 26/04/2020 17.17, Alan Third wrote:
> I’m happy to give this a go, but I’m not sure about base64 encoding.
> Do we already have a way of doing that to a char buffer?
We have base64-encode-region and base64-encode-string, which are both C functions — but why do we need to base64 encode?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Mon, 27 Apr 2020 02:29:01 GMT)
Full text and
rfc822 format available.
Message #65 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 26 Apr 2020 22:17:41 +0100
> From: Alan Third <alan <at> idiocy.org>
>
> I’m happy to give this a go, but I’m not sure about base64 encoding.
> Do we already have a way of doing that to a char buffer?
I don't think I understand the question. Can you elaborate?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Mon, 27 Apr 2020 15:23:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sun, Apr 26, 2020 at 06:48:09PM -0400, Clément Pit-Claudel wrote:
> On 26/04/2020 17.17, Alan Third wrote:
> > I’m happy to give this a go, but I’m not sure about base64 encoding.
> > Do we already have a way of doing that to a char buffer?
>
> We have base64-encode-region and base64-encode-string, which are
> both C functions — but why do we need to base64 encode?
I could be wrong, but in order to wrap the SVG file in another SVG, we
need to do something like:
<svg xmlns:xlink="http://www.w3.org/1999/xlink"
xmlns:xi="http://www.w3.org/2001/XInclude"
width="100" height="100"
preserveAspectRatio="none"
viewBox="0 0 283.465 283.465">
<xi:include href="data:image/svg+xml;base64,<base64 encoded data>">
</xi:include>
</svg>
We can’t just put the contents in directly, like:
<svg xmlns:xlink="http://www.w3.org/1999/xlink"
width="100" height="100"
preserveAspectRatio="none"
viewBox="0 0 283.465 283.465">
<!-- file contents -->
<svg>
stuff
</svg>
<!-- end file contents -->
</svg>
as the file may (and probably should) have XML declarations at the
start which break the SVG file. We could strip them out but I’m not
sure that that’s better than base64 encoding, as it would mean
modifying the contents and I’m not sure that’s the only thing that
might cause problems.
We could also potentially just insert the file as‐is like so:
<svg xmlns:xlink="http://www.w3.org/1999/xlink"
xmlns:xi="http://www.w3.org/2001/XInclude"
width="100" height="100"
preserveAspectRatio="none"
viewBox="0 0 283.465 283.465">
<xi:include href="data:image/svg+xml;utf8,<svg>stuff</svg>">
</xi:include>
</svg>
but I can’t make that work. I think we’d have to URL encode the angle
brackets and any quotes.
If there’s anything obvious I’ve missed, please let me know.
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Mon, 27 Apr 2020 15:48:01 GMT)
Full text and
rfc822 format available.
Message #71 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> From: Pip Cet <pipcet <at> gmail.com>
> Date: Sun, 26 Apr 2020 19:00:36 +0000
> Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org
>
> > Then I guess we are talking about two different use cases.
>
> A more general one, and a more limited and specific one. As it
> happens, the fix for the more general one is simpler.
"More general" in what sense? Using Lisp does mean you could write
more general programs, but it doesn't necessarily mean you will be
able to solve more general problems. The display code maintains a lot
of state that describes each position on the screen (see 'struct it'
in dispextern.h), and your proposal seems to expose only the font and
the face attributes to the Lisp function. What about the other
information? are you suggesting to expose that as well? For example,
some use cases may wish to know the pixel coordinates of the image
being rendered, or whether it's on a continued or an R2L line, or the
ascent or descent of the screen line, or the pixel distance from the
window edge, etc. etc. The display code has all of that at its
fingertips.
> > > and enhancing image specs to handle such context-dependent
> > > glyphs is wrong, because it moves code into the image backend that
> > > applies to all image backends.
> >
> > "Image backend" in this context is the image libraries, like librsvg
> > and libpng, is that right?
>
> Yes.
That's what I thought. Then I don't think I understand your comment
about "moving code into the image backend", since I never proposed to
move anything into those libraries. We need to force those of the
libraries that are capable to produce the images we want. Like force
a certain background of an SVG image. That is all done outside of the
backends.
> > A Lisp program that prepares such display can prepare the spec
> > accordingly, if needed.
>
> That's what I'm proposing: call a Lisp program to prepare a spec
> whenever we display the glyph. Not more often, which is wasteful.
I meant a Lisp program that prepared the spec before redisplay was
entered. There's no need to re-evaluate it each time the
corresponding part of the screen is updated or Emacs needs to
calculate its layout for other purposes, such as moving the cursor N
lines down on the screen. Please consider how many times this code
will be executed, sometimes in contexts that are completely unrelated
to showing the images.
> Except you're currently wrong: a Lisp program cannot prepare "the
> spec" accordingly, because there might be several required specs in
> effect at the same time, for example.
We should identify the aspects of the images we'd like to control, and
enhance the image display to obey those aspects when needed. This is
not complicated enough to call for a Lisp implementation, and doesn't
justify the downsides of calling Lisp from the display code.
> My impression is you're treating extensibility itself as something to
> be avoided, not as something on the benefit side of a cost-benefit
> analysis (cost: 5 lines of actual code. benefit: solves both the more
> specific use case sought here and the more general use case I need).
I indeed prefer to avoid extending the display engine in Lisp, as
explained elsewhere. I hope I convinced you, or that at least you see
my point. Other extensions of the display code are much more welcome,
although the resulting complexity should be carefully considered and
weighed against the advantages. The display code is extremely complex
and handles an almost infinite number of use cases, so much so that it
sometimes takes years to find bugs in some subtle use case. We should
try not to over-complicate the code without a good reason. And a
theoretical "generalization" is not a good reason in my book.
> As to how they arise: mathematicians, for example, have a
> long-standing tradition of making up symbols. Those new symbols aren't
> (and shouldn't be) in Unicode, and there are many other symbols that,
> for example, cannot be in Unicode for legal or ethical reasons. Users
> should be able to display such symbols in an Emacs buffer and have
> them be as close to character glyphs (or different from them) as they
> are willing to make them.
Could you please show examples of such symbols, and tell how they are
displayed by other editors or word processors?
> I remain convinced that I'm not smart enough to figure out in advance
> the precise set of use cases I want a feature for. You shouldn't limit
> generality to handle only those cases you're convinced are useful.
I don't want to limit generality, I want to limit the price we pay for
a use case that has yet to materialize. I think this keeps the Emacs
display code from becoming completely unmanageable.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Mon, 27 Apr 2020 16:05:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On 27/04/2020 11.22, Alan Third wrote:
> On Sun, Apr 26, 2020 at 06:48:09PM -0400, Clément Pit-Claudel wrote:
>> On 26/04/2020 17.17, Alan Third wrote:
>>> I’m happy to give this a go, but I’m not sure about base64 encoding.
>>> Do we already have a way of doing that to a char buffer?
>>
>> We have base64-encode-region and base64-encode-string, which are
>> both C functions — but why do we need to base64 encode?
>
> We can’t just put the contents in directly […]
> as the file may (and probably should) have XML declarations at the
> start which break the SVG file. We could strip them out but I’m not
> sure that that’s better than base64 encoding, as it would mean
> modifying the contents and I’m not sure that’s the only thing that
> might cause problems.
Excellent point, I hadn't considered the xml declaration at the top.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 03 May 2020 14:14:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sun, Apr 26, 2020 at 06:48:09PM -0400, Clément Pit-Claudel wrote:
> On 26/04/2020 17.17, Alan Third wrote:
> > I’m happy to give this a go, but I’m not sure about base64 encoding.
> > Do we already have a way of doing that to a char buffer?
>
> We have base64-encode-region and base64-encode-string, which are both C functions — but why do we need to base64 encode?
I’ve run into a problem with the base64 encoding.
/usr/bin/base64 encodes this file differently from Emacs:
https://upload.wikimedia.org/wikipedia/commons/9/92/MixedUnitAddition.svg
librsvg doesn’t like the way Emacs encodes the file, but is fine with
the system base64.
I think the problem is the GBP £ symbol on line 39.
Do I need to do something special, like set up character encodings?
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 03 May 2020 14:19:02 GMT)
Full text and
rfc822 format available.
Message #80 received at 40845 <at> debbugs.gnu.org (full text, mbox):
Alan Third <alan <at> idiocy.org> writes:
> I’ve run into a problem with the base64 encoding.
>
> /usr/bin/base64 encodes this file differently from Emacs:
>
> https://upload.wikimedia.org/wikipedia/commons/9/92/MixedUnitAddition.svg
>
> librsvg doesn’t like the way Emacs encodes the file, but is fine with
> the system base64.
>
> I think the problem is the GBP £ symbol on line 39.
>
> Do I need to do something special, like set up character encodings?
Before base64-encoding, you should encode the data to whatever coding
system is expected, which would probably be utf-8 here.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 03 May 2020 16:09:02 GMT)
Full text and
rfc822 format available.
Message #83 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 3 May 2020 15:13:48 +0100
> From: Alan Third <alan <at> idiocy.org>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
>
> I’ve run into a problem with the base64 encoding.
>
> /usr/bin/base64 encodes this file differently from Emacs:
>
> https://upload.wikimedia.org/wikipedia/commons/9/92/MixedUnitAddition.svg
>
> librsvg doesn’t like the way Emacs encodes the file, but is fine with
> the system base64.
>
> I think the problem is the GBP £ symbol on line 39.
>
> Do I need to do something special, like set up character encodings?
You need to make sure the text passed to base64-encode-region is
unibyte. I suggest to look at how other packages call that function.
Let me know if this general advice doesn't help.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 03 May 2020 16:25:02 GMT)
Full text and
rfc822 format available.
Message #86 received at 40845 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Sun, May 03, 2020 at 07:07:56PM +0300, Eli Zaretskii wrote:
> > Date: Sun, 3 May 2020 15:13:48 +0100
> > From: Alan Third <alan <at> idiocy.org>
> > Cc: Eli Zaretskii <eliz <at> gnu.org>, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
> >
> > I’ve run into a problem with the base64 encoding.
> >
> > /usr/bin/base64 encodes this file differently from Emacs:
> >
> > https://upload.wikimedia.org/wikipedia/commons/9/92/MixedUnitAddition.svg
> >
> > librsvg doesn’t like the way Emacs encodes the file, but is fine with
> > the system base64.
> >
> > I think the problem is the GBP £ symbol on line 39.
> >
> > Do I need to do something special, like set up character encodings?
>
> You need to make sure the text passed to base64-encode-region is
> unibyte. I suggest to look at how other packages call that function.
>
> Let me know if this general advice doesn't help.
Yes, it’s sorted the problem. Thanks!
I’ve attached a first pass. It solves 1 and 4 from the original bug
report: scaling and setting the foreground colour.
One thing is that it gets the foreground colour from the default face
instead of the current face. I’m not sure how to solve that.
I’m unsure if my use of malloc is OK, as I know Emacs code uses some
different methods in different places.
--
Alan Third
[0001-Set-basic-SVG-attributes-bug-40845.patch (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 03 May 2020 16:50:01 GMT)
Full text and
rfc822 format available.
Message #89 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 3 May 2020 17:24:17 +0100
> From: Alan Third <alan <at> idiocy.org>
> Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
>
> I’ve attached a first pass. It solves 1 and 4 from the original bug
> report: scaling and setting the foreground colour.
Thanks.
> One thing is that it gets the foreground colour from the default face
> instead of the current face. I’m not sure how to solve that.
Can you describe the difficulty in using the current face?
> I’m unsure if my use of malloc is OK, as I know Emacs code uses some
> different methods in different places.
It's okay to call malloc and free in image.c.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 03 May 2020 18:39:01 GMT)
Full text and
rfc822 format available.
Message #92 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sun, May 03, 2020 at 07:49:45PM +0300, Eli Zaretskii wrote:
> > Date: Sun, 3 May 2020 17:24:17 +0100
> > From: Alan Third <alan <at> idiocy.org>
> > Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
> >
> > One thing is that it gets the foreground colour from the default face
> > instead of the current face. I’m not sure how to solve that.
>
> Can you describe the difficulty in using the current face?
I’m mostly unsure how I access it. I don’t think images or image.c
have any real concept of where they are in the buffer.
I think we’d also have to recreate the image any time the face
changed. For example in Clement’s test code from the original bug
report, he has the face changing when the mouse hovers over the text.
In order to handle that case we would need to create a new version of
the image with the updated foreground and background when the mouse
moves over it.
We can work around having to update the background by using proper
transparency, but I don’t know what other problems that might
introduce. It’s harder for the foreground. Pip Cet suggested creating
masks for foreground and background and using them to draw the image
in sections, but I feel that’s perhaps getting a little complex,
especially when I think most images won’t be changing their foreground
and background colours.
I feel it would be easier just to have two or more cached images with
different foreground and backgrounds as required.
> > I’m unsure if my use of malloc is OK, as I know Emacs code uses some
> > different methods in different places.
>
> It's okay to call malloc and free in image.c.
Excellent, thanks.
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 03 May 2020 19:19:01 GMT)
Full text and
rfc822 format available.
Message #95 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 3 May 2020 19:38:39 +0100
> From: Alan Third <alan <at> idiocy.org>
> Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
>
> > > One thing is that it gets the foreground colour from the default face
> > > instead of the current face. I’m not sure how to solve that.
> >
> > Can you describe the difficulty in using the current face?
>
> I’m mostly unsure how I access it. I don’t think images or image.c
> have any real concept of where they are in the buffer.
When the 'load' method (in your case, svg_load) is called from
prepare_image_for_display, we have the face information handy.
Usually, at that point we don't call the 'load' method, because the
image is already loaded and cached. But we could change the logic if
we know that the image must be modified to have a different face.
Then we can pass the color information to the 'load' method.
> I think we’d also have to recreate the image any time the face
> changed.
Yes, because changing the color(s) makes a different image. But that
is not a problem, I think.
> I feel it would be easier just to have two or more cached images with
> different foreground and backgrounds as required.
Agreed. At least as the first approximation; we could always change
that later if we find this not to be good enough.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 09 May 2020 14:28:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sun, May 03, 2020 at 05:24:17PM +0100, Alan Third wrote:
>
> I’ve attached a first pass. It solves 1 and 4 from the original bug
> report: scaling and setting the foreground colour.
Can someone please try the patch from the previous email on a non‐NS
platform and confirm whether or not the background colour in the image
matches the background colour of the text?
They don’t match here, but I strongly suspect that’s because the NS
port uses different colour spaces for images and everything else.
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 09 May 2020 19:55:02 GMT)
Full text and
rfc822 format available.
Message #101 received at 40845 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Sat, May 09, 2020 at 03:27:27PM +0100, Alan Third wrote:
> On Sun, May 03, 2020 at 05:24:17PM +0100, Alan Third wrote:
> >
> > I’ve attached a first pass. It solves 1 and 4 from the original bug
> > report: scaling and setting the foreground colour.
>
> Can someone please try the patch from the previous email on a non‐NS
> platform and confirm whether or not the background colour in the image
> matches the background colour of the text?
>
> They don’t match here, but I strongly suspect that’s because the NS
> port uses different colour spaces for images and everything else.
Apologies, please can someone try the attached patch.
--
Alan Third
[0001-Set-basic-SVG-attributes-bug-40845.patch (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Fri, 15 May 2020 11:11:01 GMT)
Full text and
rfc822 format available.
Message #104 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 9 May 2020 20:54:15 +0100
> From: Alan Third <alan <at> idiocy.org>
>
> On Sat, May 09, 2020 at 03:27:27PM +0100, Alan Third wrote:
> > On Sun, May 03, 2020 at 05:24:17PM +0100, Alan Third wrote:
> > >
> > > I’ve attached a first pass. It solves 1 and 4 from the original bug
> > > report: scaling and setting the foreground colour.
> >
> > Can someone please try the patch from the previous email on a non‐NS
> > platform and confirm whether or not the background colour in the image
> > matches the background colour of the text?
> >
> > They don’t match here, but I strongly suspect that’s because the NS
> > port uses different colour spaces for images and everything else.
>
> Apologies, please can someone try the attached patch.
I didn't yet have time to try that, but I haven't forgot. Hope the
current tsunami on emacs-devel will soon calm down, and I will have
more time.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Fri, 15 May 2020 21:42:02 GMT)
Full text and
rfc822 format available.
Message #107 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Fri, May 15, 2020 at 02:09:56PM +0300, Eli Zaretskii wrote:
> > Date: Sat, 9 May 2020 20:54:15 +0100
> > From: Alan Third <alan <at> idiocy.org>
> >
> > On Sat, May 09, 2020 at 03:27:27PM +0100, Alan Third wrote:
> > > On Sun, May 03, 2020 at 05:24:17PM +0100, Alan Third wrote:
> > > >
> > > > I’ve attached a first pass. It solves 1 and 4 from the original bug
> > > > report: scaling and setting the foreground colour.
> > >
> > > Can someone please try the patch from the previous email on a non‐NS
> > > platform and confirm whether or not the background colour in the image
> > > matches the background colour of the text?
> > >
> > > They don’t match here, but I strongly suspect that’s because the NS
> > > port uses different colour spaces for images and everything else.
> >
> > Apologies, please can someone try the attached patch.
>
> I didn't yet have time to try that, but I haven't forgot. Hope the
> current tsunami on emacs-devel will soon calm down, and I will have
> more time.
Take as long as you need, I'm in no great hurry. I still need to work
out how to calculate whether I should be using the mouse face and
decide exactly how to handle flushing an image from the cache as
it's somewhat broken in that patch.
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 22 Aug 2020 16:16:01 GMT)
Full text and
rfc822 format available.
Message #110 received at 40845 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Fri, May 15, 2020 at 11:40:53PM +0200, Alan Third wrote:
>
> Take as long as you need, I'm in no great hurry. I still need to work
> out how to calculate whether I should be using the mouse face and
> decide exactly how to handle flushing an image from the cache as
> it's somewhat broken in that patch.
I still don't know how to use the mouse face. I couldn't see any way
to detect if it's in use when we first load the image in xdisp.c.
Most likely I've missed something, but if not then I worry that the
easiest fix may be to actually support transparency when we draw to
the screen instead of generating background colours when we load the
images. I suspect we do it the way we do because it would have been
slow on older machines, but modern machines should be able to handle
it quickly via XRender and friends.
As for the cache I decided to just delete all images that match the
image spec, no matter what colours they use. It seems to me to be the
safest option, as we don't want users flushing an image from the
cache to display an updated version, only for the old one to reappear
when the face changes.
--
Alan Third
[0001-Set-basic-SVG-attributes-bug-40845.patch (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 22 Aug 2020 16:30:02 GMT)
Full text and
rfc822 format available.
Message #113 received at 40845 <at> debbugs.gnu.org (full text, mbox):
Alan Third <alan <at> idiocy.org> writes:
> As for the cache I decided to just delete all images that match the
> image spec, no matter what colours they use. It seems to me to be the
> safest option, as we don't want users flushing an image from the
> cache to display an updated version, only for the old one to reappear
> when the face changes.
Makes sense.
I've tried your patch set, and it fixes the test case in this bug report
for me (under Debian).
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 22 Aug 2020 16:56:02 GMT)
Full text and
rfc822 format available.
Message #116 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 22 Aug 2020 18:15:15 +0200 (CEST)
> From: Alan Third <alan <at> idiocy.org>
>
> I still don't know how to use the mouse face. I couldn't see any way
> to detect if it's in use when we first load the image in xdisp.c.
Can you please remind me what was the problem? The bug discussion is
very long, and I didn't have time/patience to find the mouse face
bits.
> -ptrdiff_t lookup_image (struct frame *, Lisp_Object);
> +ptrdiff_t lookup_image (struct frame *, Lisp_Object, int face_id);
^^^^^^^^^^^
Please don't use names in prototypes, only types.
> + /* Parse the unmodified SVG data so we can get it's initial size. */
^^^^
"its"
> + /* The parsing is complete, rsvg_handle is ready to used, close it
^^^^^^^^^^^^^^^^
"is ready to be used"
> + background color, before including the original image. This
^^
Two spaces between sentences, please.
> + Lisp_Object encoded_contents = Fbase64_encode_string
> + (make_unibyte_string (contents, size), Qt);
Our style of breaking long lines like this one is different:
Lisp_Object encoded_contents
= Fbase64_encode_string (make_unibyte_string (contents, size), Qt);
> + if (!NILP (value))
> + {
> + foreground = image_alloc_image_color (f, img, value, img->face_foreground);
> + }
No need for braces when the block has only one line.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 22 Aug 2020 18:59:02 GMT)
Full text and
rfc822 format available.
Message #119 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sat, Aug 22, 2020 at 07:54:48PM +0300, Eli Zaretskii wrote:
> > Date: Sat, 22 Aug 2020 18:15:15 +0200 (CEST)
> > From: Alan Third <alan <at> idiocy.org>
> >
> > I still don't know how to use the mouse face. I couldn't see any way
> > to detect if it's in use when we first load the image in xdisp.c.
>
> Can you please remind me what was the problem? The bug discussion is
> very long, and I didn't have time/patience to find the mouse face
> bits.
No problem.
Basically, if you try Clement's original example, when you move the
mouse over the image the background colour doesn't change to match the
rest of the line.
I'm picking up the background colour from the face stored in "it" in
push_prefix_prop or handle_single_display_spec in xdisp.c. As far as I
can tell one or both of these are the only places we can define the
image with the face colours as this is where we actually load the
image. The problem is that I don't see any way to work out if the
current image should use the background colour from the mouse face.
I know that later on we can do (s->hl == DRAW_MOUSE_FACE), but I don't
think those structures are built yet?
I've fixed the mistakes you pointed out. Thanks.
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 22 Aug 2020 19:19:01 GMT)
Full text and
rfc822 format available.
Message #122 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 22 Aug 2020 20:57:59 +0200 (CEST)
> From: Alan Third <alan <at> idiocy.org>
> Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
>
> Basically, if you try Clement's original example, when you move the
> mouse over the image the background colour doesn't change to match the
> rest of the line.
>
> I'm picking up the background colour from the face stored in "it" in
> push_prefix_prop or handle_single_display_spec in xdisp.c. As far as I
> can tell one or both of these are the only places we can define the
> image with the face colours as this is where we actually load the
> image. The problem is that I don't see any way to work out if the
> current image should use the background colour from the mouse face.
>
> I know that later on we can do (s->hl == DRAW_MOUSE_FACE), but I don't
> think those structures are built yet?
I think you want to modify note_mouse_highlight so that it sets up the
frame's mouse-highlight info for image glyphs, like it currently does
for character glyphs. It's possible this is all that needs to be
done; if not, we should see how to change show_mouse_face and its
subroutines to display images with mouse-highlight.
Let me know if you need more guidance.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sat, 22 Aug 2020 21:36:01 GMT)
Full text and
rfc822 format available.
Message #125 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sat, Aug 22, 2020 at 10:17:53PM +0300, Eli Zaretskii wrote:
> > Date: Sat, 22 Aug 2020 20:57:59 +0200 (CEST)
> > From: Alan Third <alan <at> idiocy.org>
> > Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
> >
> > Basically, if you try Clement's original example, when you move the
> > mouse over the image the background colour doesn't change to match the
> > rest of the line.
> >
> > I'm picking up the background colour from the face stored in "it" in
> > push_prefix_prop or handle_single_display_spec in xdisp.c. As far as I
> > can tell one or both of these are the only places we can define the
> > image with the face colours as this is where we actually load the
> > image. The problem is that I don't see any way to work out if the
> > current image should use the background colour from the mouse face.
> >
> > I know that later on we can do (s->hl == DRAW_MOUSE_FACE), but I don't
> > think those structures are built yet?
>
> I think you want to modify note_mouse_highlight so that it sets up the
> frame's mouse-highlight info for image glyphs, like it currently does
> for character glyphs. It's possible this is all that needs to be
> done; if not, we should see how to change show_mouse_face and its
> subroutines to display images with mouse-highlight.
>
> Let me know if you need more guidance.
Thanks for your help, and that certainly helped me understand a bit
more about how mouse highlighting works and I don't think setting the
background colour from a face is a practical way of doing this.
But I've just realised we don't need to do it. Masks provide this
functionality. If you want the background of your SVG to match the
background of the buffer, use a mask, which can be keyed off a
specific colour or whatever.
Unfortunately it won't work well for semi-transparent images, but I
think the only really practical solution for that is to introduce
proper transparency handling.
Therefore I think this patch is ready to go.
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 23 Aug 2020 05:48:01 GMT)
Full text and
rfc822 format available.
Message #128 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 22 Aug 2020 23:35:46 +0200 (CEST)
> From: Alan Third <alan <at> idiocy.org>
> Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
>
> Therefore I think this patch is ready to go.
Thanks. Before we install this, one question: is there a possibility
someone would want the previous behavior?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 23 Aug 2020 09:10:01 GMT)
Full text and
rfc822 format available.
Message #131 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sun, Aug 23, 2020 at 08:47:27AM +0300, Eli Zaretskii wrote:
> > Date: Sat, 22 Aug 2020 23:35:46 +0200 (CEST)
> > From: Alan Third <alan <at> idiocy.org>
> > Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
> >
> > Therefore I think this patch is ready to go.
>
> Thanks. Before we install this, one question: is there a possibility
> someone would want the previous behavior?
I don't think so. The resizing is plainly better, and if someone wants
to enforce a foreground and background colour they can still do that
with the :foreground and :background image properties.
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 23 Aug 2020 09:13:02 GMT)
Full text and
rfc822 format available.
Message #134 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 23 Aug 2020 11:09:01 +0200 (CEST)
> From: Alan Third <alan <at> idiocy.org>
> Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
>
> > Thanks. Before we install this, one question: is there a possibility
> > someone would want the previous behavior?
>
> I don't think so. The resizing is plainly better, and if someone wants
> to enforce a foreground and background colour they can still do that
> with the :foreground and :background image properties.
OK, then let's say that last bit in the NEWS entry announcing this new
behavior.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 23 Aug 2020 11:49:02 GMT)
Full text and
rfc822 format available.
Message #137 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sun, Aug 23, 2020 at 12:11:55PM +0300, Eli Zaretskii wrote:
> > Date: Sun, 23 Aug 2020 11:09:01 +0200 (CEST)
> > From: Alan Third <alan <at> idiocy.org>
> > Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
> >
> > > Thanks. Before we install this, one question: is there a possibility
> > > someone would want the previous behavior?
> >
> > I don't think so. The resizing is plainly better, and if someone wants
> > to enforce a foreground and background colour they can still do that
> > with the :foreground and :background image properties.
>
> OK, then let's say that last bit in the NEWS entry announcing this new
> behavior.
Something like this?
*** The background and foreground of images now default to face colors.
To load images with the default frame colors use the ':foreground'
and ':background' image attributes, for example:
(create-image "filename" nil nil
:foreground (face-attribute 'default :foreground)
:background (face-attribute 'default :background))
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 23 Aug 2020 12:06:01 GMT)
Full text and
rfc822 format available.
Message #140 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 23 Aug 2020 13:48:45 +0200 (CEST)
> From: Alan Third <alan <at> idiocy.org>
> Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
>
> *** The background and foreground of images now default to face colors.
> To load images with the default frame colors use the ':foreground'
> and ':background' image attributes, for example:
>
> (create-image "filename" nil nil
> :foreground (face-attribute 'default :foreground)
> :background (face-attribute 'default :background))
Yes, thanks. But maybe expand a bit about "the face colors" part (not
in the heading line, but in the body): what face is that, and when
will the colors be used (I understand they will be used only if the
image doesn't specify its own colors).
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 23 Aug 2020 12:20:02 GMT)
Full text and
rfc822 format available.
Message #143 received at 40845 <at> debbugs.gnu.org (full text, mbox):
On Sun, Aug 23, 2020 at 03:05:24PM +0300, Eli Zaretskii wrote:
>
> Yes, thanks. But maybe expand a bit about "the face colors" part (not
> in the heading line, but in the body): what face is that, and when
> will the colors be used (I understand they will be used only if the
> image doesn't specify its own colors).
How's this?
---
*** The background and foreground of images now default to face
colors. When an image doesn't specify a foreground or background
color, Emacs now uses colors from the face used to draw the
surrounding text instead of the frame's default colors.
To load images with the default frame colors use the ':foreground'
and ':background' image attributes, for example:
(create-image "filename" nil nil
:foreground (face-attribute 'default :foreground)
:background (face-attribute 'default :background))
This change doesn't affect image types that do not support foreground
and background colors, such as png or jpeg.
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 23 Aug 2020 12:24:01 GMT)
Full text and
rfc822 format available.
Message #146 received at 40845 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 23 Aug 2020 14:19:28 +0200 (CEST)
> From: Alan Third <alan <at> idiocy.org>
> Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
>
> *** The background and foreground of images now default to face
> colors. When an image doesn't specify a foreground or background
> color, Emacs now uses colors from the face used to draw the
> surrounding text instead of the frame's default colors.
>
> To load images with the default frame colors use the ':foreground'
> and ':background' image attributes, for example:
>
> (create-image "filename" nil nil
> :foreground (face-attribute 'default :foreground)
> :background (face-attribute 'default :background))
>
> This change doesn't affect image types that do not support foreground
> and background colors, such as png or jpeg.
Almost perfect: I would suggest to modify the last sentence so that it
tells which types of images do support this feature.
Thanks.
Reply sent
to
Alan Third <alan <at> idiocy.org>
:
You have taken responsibility.
(Sun, 23 Aug 2020 15:30:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Clément Pit-Claudel <cpitclaudel <at> gmail.com>
:
bug acknowledged by developer.
(Sun, 23 Aug 2020 15:30:02 GMT)
Full text and
rfc822 format available.
Message #151 received at 40845-done <at> debbugs.gnu.org (full text, mbox):
On Sun, Aug 23, 2020 at 03:23:33PM +0300, Eli Zaretskii wrote:
> > Date: Sun, 23 Aug 2020 14:19:28 +0200 (CEST)
> > From: Alan Third <alan <at> idiocy.org>
> > Cc: cpitclaudel <at> gmail.com, 40845 <at> debbugs.gnu.org, pipcet <at> gmail.com
> >
> > *** The background and foreground of images now default to face
> > colors. When an image doesn't specify a foreground or background
> > color, Emacs now uses colors from the face used to draw the
> > surrounding text instead of the frame's default colors.
> >
> > To load images with the default frame colors use the ':foreground'
> > and ':background' image attributes, for example:
> >
> > (create-image "filename" nil nil
> > :foreground (face-attribute 'default :foreground)
> > :background (face-attribute 'default :background))
> >
> > This change doesn't affect image types that do not support foreground
> > and background colors, such as png or jpeg.
>
> Almost perfect: I would suggest to modify the last sentence so that it
> tells which types of images do support this feature.
Thanks. I've pushed it to master.
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 23 Aug 2020 15:44:02 GMT)
Full text and
rfc822 format available.
Message #154 received at 40845-done <at> debbugs.gnu.org (full text, mbox):
Alan Third <alan <at> idiocy.org> writes:
> Thanks. I've pushed it to master.
Great!
I'm getting this warning, though:
image.c: In function ‘lookup_image’:
image.c:2340:17: warning: potential null pointer dereference [-Wnull-dereference]
2340 | unsigned long background = FACE_COLOR_TO_PIXEL (face->background, f);
| ^~~~~~~~~~
image.c:2339:17: warning: potential null pointer dereference [-Wnull-dereference]
2339 | unsigned long foreground = FACE_COLOR_TO_PIXEL (face->foreground, f);
| ^~~~~~~~~~
This is on Debian/bullseye, with gcc (Debian 9.3.0-15) 9.3.0.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 23 Aug 2020 16:09:02 GMT)
Full text and
rfc822 format available.
Message #157 received at 40845-done <at> debbugs.gnu.org (full text, mbox):
On Sun, Aug 23, 2020 at 05:43:24PM +0200, Lars Ingebrigtsen wrote:
> Alan Third <alan <at> idiocy.org> writes:
>
> > Thanks. I've pushed it to master.
>
> Great!
>
> I'm getting this warning, though:
>
> image.c: In function ‘lookup_image’:
> image.c:2340:17: warning: potential null pointer dereference [-Wnull-dereference]
> 2340 | unsigned long background = FACE_COLOR_TO_PIXEL (face->background, f);
> | ^~~~~~~~~~
> image.c:2339:17: warning: potential null pointer dereference [-Wnull-dereference]
> 2339 | unsigned long foreground = FACE_COLOR_TO_PIXEL (face->foreground, f);
> | ^~~~~~~~~~
>
> This is on Debian/bullseye, with gcc (Debian 9.3.0-15) 9.3.0.
I don't think I should have used FACE_FROM_ID_OR_NULL. I would hope
we're not going to be trying to load images without a default face
already defined.
I've pushed a fix.
--
Alan Third
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#40845
; Package
emacs
.
(Sun, 23 Aug 2020 16:39:02 GMT)
Full text and
rfc822 format available.
Message #160 received at 40845-done <at> debbugs.gnu.org (full text, mbox):
Alan Third <alan <at> idiocy.org> writes:
> I don't think I should have used FACE_FROM_ID_OR_NULL. I would hope
> we're not going to be trying to load images without a default face
> already defined.
>
> I've pushed a fix.
Thanks; that fixes the warnings.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Mon, 21 Sep 2020 11:24:03 GMT)
Full text and
rfc822 format available.
This bug report was last modified 4 years and 270 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.