[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
From: |
Eli Zaretskii |
Subject: |
bug#35468: [PATCH] Refactor draw_glyph_string on X and w32 |
Date: |
Thu, 02 May 2019 23:14:55 +0300 |
> From: Alex Gramiak <agrambot@gmail.com>
> Cc: 35468@debbugs.gnu.org
> Date: Thu, 02 May 2019 13:41:56 -0600
It's late down here, and I had a bad week and an even worse day. So
just a few quick answers for now, and the rest will follow in a few
hours.
> > It is slightly inelegant, but it certainly isn't a catastrophe.
> > Unused arguments that are there for the sake of a function signature
> > compatibility is not an uncommon technique in C.
>
> I suppose. It's only because of the w32 version that uses s->HDC that
> `s' needs to be passed in at all for the glyph string version.
Why can't you pass HDC itself? It's just a pointer.
> > But please note that gc->foreground and background used in some cases
> > are exactly the color numbers used directly in other cases, so I think
> > you could always pass colors or always pass a GC.
>
> Do you mean that I could leave out either the color or the GC argument?
Yes, that was my impression from looking at the code. We will just
need to pack the color into a GC in some cases.
> >> 4) The w32 versions of some procedures need to save the font around the
> >> calls to font->driver->draw; is this necessary?
> >
> > Yes. The X 'draw' methods accept the font as an argument, but the w32
> > implementation relies on setting the font outside of the 'draw' method
> > because the 'draw' method draws using the "currently selected font".
> > Then we need to restore the original font.
>
> Where do the X 'draw' methods accept the font as an argument? Looking
> at, e.g., *_draw_glyph_string_foreground, font->driver->draw takes the
> same arguments.
Look at the driver's 'draw' method itself, e.g. xftfont_draw.
> Since font->driver->draw takes in the glyph string, why can't the 'draw'
> methods use SelectObject (s->hdc, FONT_HANDLER (s->font)) and
> SelectObject (s->hdc, old_font)?
It could, but that would be less efficient, since we many times call
the draw method several times in a row.
> If they can't, is there any other way to do it inside the draw methods?
> It seems like it would simplify the code on the w32 side and make it
> more in line with the other backends.
I would leave this for another rainy day, and for now just have an
interface which just w32 will implement.
> > You could define a set_font interface, with only a w32 implementation.
>
> Did you have something else in mind besides the
> save/restore_secondary_context_font I posted?
Not besides, instead.
> >> if (gdif->save_secondary_context_font)
> >
> > This name is misleading: this is not a "secondary" font. We are
> > selecting the font to be used for drawing by the font driver's 'draw'
> > method.
>
> The "secondary" here applies to "context", not "context font". I used
> the name since the code changes s->hdc's (what I dubbed to be the
> "secondary context") font. Would you prefer just
> {save,restore}_context_font?
There's no context here, either. That call selects a different font,
and returns a handle of the previous font so that it could be restored
later.
Stay tuned for the rest.
Thanks.
- bug#35468: [PATCH] Refactor draw_glyph_string on X and w32, Eli Zaretskii, 2019/05/01
- bug#35468: [PATCH] Refactor draw_glyph_string on X and w32, Eli Zaretskii, 2019/05/01
- bug#35468: [PATCH] Refactor draw_glyph_string on X and w32, Alex Gramiak, 2019/05/02
- bug#35468: [PATCH] Refactor draw_glyph_string on X and w32, Eli Zaretskii, 2019/05/04
- bug#35468: [PATCH] Refactor draw_glyph_string on X and w32, Alex Gramiak, 2019/05/04
- bug#35468: [PATCH] Refactor draw_glyph_string on X and w32, mituharu, 2019/05/04
- bug#35468: [PATCH] Refactor draw_glyph_string on X and w32, Eli Zaretskii, 2019/05/05
- bug#35468: [PATCH] Refactor draw_glyph_string on X and w32, Eli Zaretskii, 2019/05/04
bug#35468: [PATCH] Refactor draw_glyph_string on X and w32, Alex Gramiak, 2019/05/03