images inside urxvt: core patch#7
Open
drzraf wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Here is a patch rerolling my previous (2013) attempt at displaying images inside urxvt, hopefully addressing all previous comments:
A. minimal impact to the codebase
B. no gdk pixbuf dependency
C. OSC definition (and layout semantics) on Perl's side
About layout (positioning/padding/inline/...):
All the format-parsing are definitely better in Perl.
IMG_EOL_FLAG(IMG_EOLPerl counterpart) defines whether an image sticks to the right most border. It's the only layout/details flag that calls forscreen.Cconsideration.Other aspects (padding/size/ratio/wrap/pos/flow) are handled by the extension.
About Perl/core separation and trade-off:
rxvt_term::scr_expose()andrxvt_term::scr_touch()+ a 10 lines core-change) but it was simply impossible to avoid glitches: Resizing, rewrapping, relationship to cursor, ... imply many sequences and states deeply nested insidesrc_*()functions: likecopy_line(),scr_scroll_text()and even morescr_refresh().In one way or another tracking images across refreshes at the cell level or clearing areas of images' previous position implied trespassing
screen.CThat why this v2 only went from 780 LoC to "only" 500 LoC (+ 60 LoC for 3 xs bindings)
Other notes:
_InTermImagewas an initial attempt to make the feature more modular and self-contained.But a better solution was to store images as attribute of a line_t were all the flags/col/width/height could live and benefit from the existing line manipulation/display/refresh logic.
The overlay code ended up not being a good fit because its purpose and mechanism are entirely different (fixed area vs cell-based/tracking logic)
Two semi-generic helpers added into
rxvt_img(no specific#if HAVE_IMAGES)Main limitation: interactivity:
The implementation assigns no ID to pixmaps.
C maintains persistence when row numbering changes but Perl barely knows the filename and can't tells whether an image exists in the viewport or under a mouse click.
Having unique IDs to bridge both sides would efficiently provide this information, allowing the Perl extension to add click handlers/action/file name copy/... features
Given the extensibility potential, it could be worth for the price of another
intinsideline_image_t? (maybe the file's inode could be used)(It would also have made the patch a bit more complex for another "extra" feature so I preferred to ignore it for now)
New bindings:
rxvt_term::has_line_image (int row_number)rxvt_term::set_line_image (int row_number, rxvt_img *img, int col = 0, int flags = 0)rxvt_term::clear_line_image (int row_number)Example:
(test sizing, aligning, inlining, left border sticking, transparency, compatibility with backgrounds and pseudo-transparency, etc...)
Posting a PR here because the mailing-list seems dead ATM. My patch got moderated (exceeding the 20kb attachment size) but there seem to be no moderator available anymore. I really wish urxvt itself is not dead.