[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
From: |
Alex Gramiak |
Subject: |
bug#35468: [PATCH] Refactor draw_glyph_string on X and w32 |
Date: |
Sat, 04 May 2019 13:29:34 -0600 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) |
Eli Zaretskii <eliz@gnu.org> writes:
>> 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.
>
> The way I wrote it was confusing: by the 'draw' method I actually
> meant the external APIs called by the 'draw' method, like
> XftDrawGlyphs. Compare that with w32's ExtTextOutW in w32font_draw.
Ah, I see. I'll keep the setter and rename it to, say,
set_device_context_font.
>> I'm having trouble with *_draw_image_foreground -- they just seem too
>> different to nicely abstract. Would it be okay if some generic
>> constructs leak into it (namely: s->img->mask)? Otherwise the common
>> setup that w32 does would be problematic.
>
> I don't think I understand the difficulties, sorry. Why is
> s->img->mask a problem?
I meant problem as in that it's "leaking" the internals a bit.
> In any case, it's not 100% "verboten" for platform-specific code to
> look at the internals of 'struct glyph_string', if the interface needs
> many different members of that struct. Avoiding this is just a
> general rule, which makes it easier to implement generic interfaces
> that will fit future uses. However draw_image (which, btw, I'd call
> draw_image_foreground) looks specialized enough to be exempt of that
> rule.
That's good; it will be easier to work with the image procedures in that
case.
>> For reference, this is what I have right now for
>> gui_draw_image_foreground:
>>
>> static void
>> gui_draw_image_foreground (struct glyph_string *s)
>> {
>> struct graphical_drawing_interface *gdif = FRAME_GDIF (s->f);
>> int x = s->x;
>> int y = s->ybase - image_ascent (s->img, s->face, &s->slice);
>>
>> /* If first glyph of S has a left box line, start drawing it to the
>> right of that line. */
>> if (s->face->box != FACE_NO_BOX
>> && s->first_glyph->left_box_line_p
>> && s->slice.x == 0)
>> x += eabs (s->face->box_line_width);
>>
>> /* If there is a margin around the image, adjust x- and y-position
>> by that margin. */
>> if (s->slice.x == 0)
>> x += s->img->hmargin;
>> if (s->slice.y == 0)
>> y += s->img->vmargin;
>>
>> if (gdif->save_secondary_context)
>> gdif->save_secondary_context (s, CON_ALL); // SaveDC (s->hdc);
>>
>> if (gdif->glyph_has_image (s))
>
> What details does glyph_has_image hide? Is that just to test
> s->img->pixmap?
On most platforms, yes, but the Cairo drawing uses s->img->cr_data
instead.
>> {
>> gdif->draw_image (s, s->img->width, s->img->height,
>> s->slice.x, s->slice.y, s->slice.width,
>> s->slice.height,
>> x, y, true);
>> if (!gdif->glyph_image_uses_mask (s))
>
> And what does glyph_image_uses_mask hide? AFAIU, the current code
> simply looks at s->img->mask, and if so, why do we need an interface
> for that?
I was thinking that since AFAIU the Cairo drawing doesn't set
s->img->mask it wouldn't make sense, from an interface POV, to check it
directly. I suppose it doesn't really matter in that case, and it would
be faster to just check s->img->mask even if the backend doesn't use it.
bug#35468: [PATCH] Refactor draw_glyph_string on X and w32, Alex Gramiak, 2019/05/03