Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add imageplot (support sixel) #196

Merged
merged 5 commits into from
Sep 2, 2022
Merged

Conversation

t-bltg
Copy link
Member

@t-bltg t-bltg commented Dec 18, 2021

Fix #153, goes with JuliaPlots/Plots.jl#4011.

with Sixel 1

using Sixel, UnicodePlots, ImageInTerminal, TestImages, ImageTransformations

main() = begin
  img = imresize(testimage("mandril_color"), ratio=.4)
  display(UnicodePlots.image(img, title="mandril sixel"))
  return
end

main()

mandril

without Sixel

using UnicodePlots, ImageInTerminal, TestImages, ImageTransformations

main() = begin
  img = imresize(testimage("mandril_color"), ratio=.4)
  ImageInTerminal.use_24bit()
  display(UnicodePlots.image(img, title="mandril 24 bit"))
  ImageInTerminal.use_256()
  display(UnicodePlots.image(img, title="mandril 256 color"))
  return
end

main()

mandril_24_256

Discussion:

  • naming ?, should we export this ? chose to unexport image
  • img is downsized using restrict in ImageInTerminal, is there a solution to get the ratio so that we can add a centered title and/or labels ? solved using terminal control sequences to query the cursor carret size
  • is the approach using @require standard, for optional dependency ImageInTerminal, or should we instead add ImageInTerminal as a hard dependency ?
  • the sixel implementation was tricky, had to use some ansi escape codes for printing the right border, maybe we can do something better (see the following mwe for writing after a sixel sequence)
$ printf '\ePq
#0;2;0;0;0#1;2;100;100;0#2;2;0;100;0
#1~~@@vv@@~~@@~~$
#2??}}GG}}??}}??-
#1!14@\e\\\e[A\e[2Cworld
'

hi

Footnotes

  1. for sixel support, code snippet must be ran under a compatible terminal.

@codecov-commenter
Copy link

codecov-commenter commented Dec 18, 2021

Codecov Report

Base: 99.94% // Head: 99.14% // Decreases project coverage by -0.79% ⚠️

Coverage data is based on head (db0a707) compared to base (cf9be69).
Patch coverage: 75.86% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #196      +/-   ##
==========================================
- Coverage   99.94%   99.14%   -0.80%     
==========================================
  Files          29       31       +2     
  Lines        1716     1755      +39     
==========================================
+ Hits         1715     1740      +25     
- Misses          1       15      +14     
Impacted Files Coverage Δ
src/UnicodePlots.jl 100.00% <ø> (ø)
src/graphics/imagegraphics.jl 65.62% <65.62%> (ø)
src/common.jl 99.09% <66.66%> (-0.45%) ⬇️
src/show.jl 99.20% <88.88%> (-0.80%) ⬇️
src/canvas/densitycanvas.jl 100.00% <100.00%> (ø)
src/graphics.jl 100.00% <100.00%> (ø)
src/graphics/bargraphics.jl 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@t-bltg t-bltg marked this pull request as draft December 19, 2021 19:20
@t-bltg t-bltg marked this pull request as ready for review December 19, 2021 21:49
@t-bltg t-bltg requested a review from johnnychen94 December 19, 2021 21:49
@t-bltg t-bltg force-pushed the img_in_terminal branch 2 times, most recently from 9184f35 to 012e344 Compare December 19, 2021 21:57
@t-bltg t-bltg changed the title Initial implementation of imshow Support images (and sixel) Dec 19, 2021
@t-bltg t-bltg force-pushed the img_in_terminal branch 10 times, most recently from 2c50ff0 to df995db Compare December 19, 2021 23:10
@johnnychen94
Copy link
Collaborator

johnnychen94 commented Dec 19, 2021

I hope we can pend this PR for a while until we address JuliaImages/ImageInTerminal.jl#48. The current ImageInTerminal has one unwanted side effect; that any package loads ImageInTerminal would change how colorant arrays are rendered in terminal even if you don't call it with UnicodePlots.image because it overrides Base.show.

@t-bltg
Copy link
Member Author

t-bltg commented Dec 20, 2021

I hope we can pend this PR for a while until we address JuliaImages/ImageInTerminal.jl#48.

Thanks, for pointing that. Maybe I can help on that matter ?

The current ImageInTerminal has one unwanted side effect; that any package loads ImageInTerminal would change how colorant arrays are rendered in terminal even if you don't call it with UnicodePlots.image because it overrides Base.show.

Is this something we want to change for ImageInTerminal 1.0 ?

@t-bltg t-bltg force-pushed the img_in_terminal branch 8 times, most recently from 071201b to 898eddd Compare December 20, 2021 19:17
@johnnychen94
Copy link
Collaborator

johnnychen94 commented Dec 20, 2021

Is this something we want to change for ImageInTerminal 1.0 ?

Yes, I'd like to separate ImageInTerminal into two packages: backends and frontends. The backend might be as simple as ascii_encode(img) and the frontend would provide a Display type and override Base.display and Base.show method. For instance, Sixel.jl can be seen as an ImageInTerminal backend.

To do this, we need to rework and deprecate the current imshow implementation: 1) introduces a pure encoding backend ascii_encode([io], img, method; kwargs...) as I did in Sixel.jl, make it a standalone package (say AsciiImageEncoding, a bad package name, but you can get the point), and 2) introduce a TerminalGraphicDisplay type in the frontend package ImageInTerminal.

By doing this, this PR can directly use the ascii_encode methods in the new backend package just like using sixel_encode in Sixel. We could also move some core encoding functionality of the UnicodePlots to the new backend package. I believe this provides better flexibility in reusing the codes.

@johnnychen94
Copy link
Collaborator

Thanks, for pointing that. Maybe I can help on that matter ?

Yeah, given that I have too many items on my todo-list it would be great if you can help implement this! I just sent you an invitation to ImageInTerminal, you can choose to implement the backend package in it as a sub-package like SnoopCompileCore, or you can create a new package and refactor the encoding part of ImageInTerminal to the new package.

@t-bltg
Copy link
Member Author

t-bltg commented Dec 20, 2021

Thanks, I'll continue the discussion in JuliaImages/ImageInTerminal.jl#52 about details.

Project.toml Outdated Show resolved Hide resolved
test/tst_imageplot.jl Show resolved Hide resolved
@t-bltg t-bltg changed the title Support images (and sixel) add imageplot (support sixel) Aug 30, 2022
@t-bltg t-bltg force-pushed the img_in_terminal branch 4 times, most recently from 4b37961 to c7a9bda Compare September 1, 2022 16:13
@t-bltg
Copy link
Member Author

t-bltg commented Sep 1, 2022

I will probably merge this in the next few days, unless objection(s).
The usage is a bit marginal with Sixels, but my usage of it atm is fine.

@t-bltg t-bltg merged commit a03412b into JuliaPlots:master Sep 2, 2022
@t-bltg t-bltg deleted the img_in_terminal branch September 2, 2022 07:33
@t-bltg t-bltg mentioned this pull request Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] Support sixel encoding
3 participants