bug#50660: 28.0.50; Text artifacting when the cursor moves over text under mouse face that originally displayed a box

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, this looks almost right to me, see the minor stylistic
> comments and questions below.  (I didn't yet have time to try the
> patch, but if you did try it in all the relevant combinations,
> including R2L text, that's good enough for me.)

I tried to make it work with RTL text, but right now I don't have access
to the machine where I developed that patch, so I wasn't able to test
it.  (For me to move code between machines at work onto my personal
machine is difficult due to the policies of my organization.  But they
have no issue with making the changes public.  Oh well, can't convince
the suits.)

I will correct the issues ASAP.

> But it looks like the code in both conditions is the same?  More
> generally, what kind of problem specific to images does this part try
> to handle?

Thanks, I think that's a problem.  The conditions should be different
depending on whether or not the row is reversed, because
produce_image_glyph uses start_of_box_run_p and end_of_box_run_p, which
AFAIU is unaffected by the row being reversed.

This part exists to take into account produce_image_glyph testing for
several conditions being met before appending the box width to the
iterator.

(See this part of produce_image_glyph, somewhere around line 29542:)

          if (it->start_of_box_run_p && slice.x == 0)
                                        ^^^^^^^^^^^^
            it->pixel_width += face->box_vertical_line_width;
          if (it->end_of_box_run_p && slice.x + slice.width == img->width)
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            it->pixel_width += face->box_vertical_line_width;

I suppose it should be made a comment.  That will be done shortly,
thanks.


>> +      if (do_left_box_p)
>> +        *offset -= max (0, regular_face->box_vertical_line_width);
>> +      /* Likewise with the right box line, as there may be a
>> +     box there as well. */
>> +      if (do_right_box_p)
>> +        *offset -= max (0, regular_face->box_vertical_line_width);

> There's no reason to use -= and += here.  The callers never initialize
> the argument to anything but zero, nor should they.  This function
> _calculates_ the offset, it doesn't _correct_ it.  So a simple
> assignment should do better, because using the above begs the
> question: what could the initial value be?  The callers should add or
> subtract the corrections as they see fit.

Yes, that one is specifically one of my vices.  Thanks for pointing it
out.