alphapapa / ement.el

A Matrix client for GNU Emacs
GNU General Public License v3.0
475 stars 44 forks source link

Image scaling #255

Closed phil-s closed 3 months ago

phil-s commented 5 months ago

Bug fixes and enhancements for image scaling.

phil-s commented 5 months ago

Currently a draft on account of "Prefix argument prompts for scale" which was under discussion in https://github.com/alphapapa/ement.el/issues/251 (for the moment I've included that change as it was in that previous code, and haven't yet put a lot of thought into alternatives).

phil-s commented 5 months ago

I've changed my mind about including #251 in this, partly because ement-room-image-show already provides a way of viewing the image at 100%. I think there are more improvements to be made in this area, but that this particular PR can move forwards without that (and I'm happy with the other commits).

phil-s commented 5 months ago

Coincidentally, I just encountered a case where a very wide image was, at its initial/reduced thumbnail height, too wide for the window body.

I could add support for that case in the same manner as the width restrictions for the "maximised" images.

(This is definitely an edge case though, so it could also be fixed later.)

phil-s commented 5 months ago

Rebased over current master.

phil-s commented 5 months ago

Coincidentally, I just encountered a case where a very wide image was, at its initial/reduced thumbnail height, too wide for the window body.

I could add support for that case in the same manner as the width restrictions for the "maximised" images.

(This is definitely an edge case though, so it could also be fixed later.)

On review, I'm not sure we have the necessary information at the time it's needed to handle this case as easily. A backtrace for ement-room--format-m.image is:

I don't think the specific buffer position for the image is available yet.

If I'm right, then I figure that all we could do here is register that (for images generally) we wish to do some post-processing after their insertion into the buffer, and run a hook for that (at the end of ement-room--pp-thing?) which would enable us to adjust the max-width image property if necessary, based on where the left-hand edge of the image had actually ended up.

(This seems more and more like a "later" thing.)

phil-s commented 3 months ago

Rebased over current master.

alphapapa commented 3 months ago

Thanks for your patience, and for fixing these problems. Merged (correctly this time)!