bug#38181: Actual height of mode-line not taken into account

[martin rudalics]

> The second call happens before the first one, so it looks more correct
> to me.  Why did you think you didn't need to set the frame title for
> child frames?

Because on all systems excluding Windows child frames do not show their
titles.  Calculating something that cannot be displayed doesn't strike
me as a good idea.  And obviously native tooltip frames which suffer the
same problem using

(progn
  (setq x-gtk-use-system-tooltips nil)
  (set-face-background 'internal-border "red"))

would still not display borders correctly.  Or should we set their
titles too?

>> Disclaimer: In all those runs I do not know whether the
>>
>>     (set-face-background 'internal-border "red")
>>
>> set the face_change flag or something else did.
>
> It doesn't.  It sets the face_change flag of each frame instead.  See
> internal-set-lisp-face-attribute.

This way of setting things confuses me.  What is then that global
face_change bool needed for?

>> It might have been a wrong conclusion on my side.  Maybe the problem is
>> caused by the fact that face_change is not set by 'set-face-background'
>> and the face change that triggered the backtraces above came from
>> somewhere else.
>
> See above.  It could be that we somehow fail to redisplay the child
> frame thoroughly enough, though.

I have not tried yet but I'm convinced that we would fail for a normal
frame as well if we didn't set its frame title.  This reliance of one
(internal border color) upon the conceptually unrelated other (setting
the frame title) looks fishy to me and IIRC causes redisplay_internal to
initially run with old character sizes until the actual ones get
realized when setting the frame title.

>> So if I set `inhibit-free-realized-faces' to nil at some arbitrary time
>> things may go wrong.
>
> Only if you do that in code that can run while redisplay_internal is
> doing its job.

That's what I meant, yes.

>>   > The other two are the reason why we set the
>>   > inhibit_free_realized_faces in PRODUCE_GLYPHS: the flag must be set
>>   > during redisplay, until update_frame finished its job.  Otherwise we
>>   > will sooner or later crash.
>>
>> I'm running without this for a couple of days now and it did not crash
>> so far.
>
> Try setting things up such that faces are modified by some Lisp that
> runs during redisplay, like some :eval form of the mode line or one of
> the hooks called by the display code.  That's how crashes with face =
> NULL invariably start.

Not really needed.  You convinced me here.

>> Sheer luck, I presume.  Couldn't PRODUCE_GLYPHS set
>> inhibit_free_realized_faces iff redisplaying_p is true?
>
> No, because some functions we call from Lisp actually write into the
> current matrix.  For example, tool-bar-height and tab-bar-height.

Then there's more to it than the above "the flag must be set during
redisplay, until update_frame finished its job"?  I'm still confused but
that's probably my poor mind.  Maybe re-reading your text after a day or
so will help.

>> So it's not so entirely trivial to do that in Elisp.
>
> Why do we need to do this in Lisp?

Didn't you propose to calculate the mode-line height from
'fit-window-to-buffer'?

>> OK.  But IIUC so far we do not allow inhibit_free_realized_faces nil
>> outside of redisplay_internal.  Unless someone sets
>> 'inhibit-free-realized-faces' which is, in general, to recommended.
>> Right?
>
> Yes, but I see no reason not to reset it in other places, provided
> that we do it carefully and only when absolutely needed.

We should say that in the doc-string of 'inhibit-free-realized-faces',
maybe.

martin