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

support DRCS Sixel Graphics #7

Merged
merged 21 commits into from
Sep 13, 2021
Merged

support DRCS Sixel Graphics #7

merged 21 commits into from
Sep 13, 2021

Conversation

mattn
Copy link
Contributor

@mattn mattn commented Mar 28, 2018

This PR add DRCS Sixel Graphics support. Current implementation doesn't handle terminal cell width/height. So It draw 8x8 size always. #6

terminal4

Screenshot is taken on Windows. But I removed Windows support since msys2 terminal doesn't have enough feature to support jplot.

@rs
Copy link
Owner

rs commented Mar 28, 2018

Can you please update the README with info about which terminal(s)/OS (with links) are supported by this feature?

@mattn
Copy link
Contributor Author

mattn commented Mar 29, 2018

Sorry, I don't understand what we should fix about cellWidth/cellHeight.

@rs
Copy link
Owner

rs commented Apr 11, 2018

Basically, instead of having a initCellSize function with a condition on the type of terminal, I would have two separate functions for each implementation in their respective file instead of common.go.

@mattn
Copy link
Contributor Author

mattn commented Apr 11, 2018

For example, separate to cellWidthIterm/cellHeightIterm, and cellWidthSixel/cellHeightSixel ?

@rs
Copy link
Owner

rs commented Apr 12, 2018

You're right, it would make Size hard to maintain. Let's keep it this way.

@rs
Copy link
Owner

rs commented Apr 12, 2018

About terminal support, I tried xterm as listed but it does not work:

root@kali:~/go/src/github.com/rs/jplot# go run main.go 
jplot:  iTerm2 or DRCS Sixel graphics required
exit status 1
root@kali:~/go/src/github.com/rs/jplot# echo $TERM
xterm

Are you referring to a specific version of xterm?

@rs
Copy link
Owner

rs commented Apr 12, 2018

This list might be more helpful: https://github.com/saitoha/PySixel#requirements

@rs
Copy link
Owner

rs commented Apr 12, 2018

With the -ti 340 option, xterm works but the 16 colors limit give an interesting result:

screen shot 2018-04-11 at 6 25 06 pm

I'm not sure we can say xterm is supported at this point.

@rs
Copy link
Owner

rs commented Apr 12, 2018

mlterm is not a lot better:
screen shot 2018-04-11 at 6 29 45 pm

@mattn
Copy link
Contributor Author

mattn commented Apr 12, 2018

Hmm, xterm/mlterm seems not render transparent colors correctly.

@mattn
Copy link
Contributor Author

mattn commented Apr 12, 2018

The area under lines are painted as black or gray? Or transparent with white?

@rs
Copy link
Owner

rs commented Apr 12, 2018

They have alpha with the line's color.

The size is also wrong. I guess it's because you can't get the cell size. There is absolutely no way to get the term size in pixels with Sixel?

@mattn
Copy link
Contributor Author

mattn commented Apr 12, 2018

This mintty terminal and running jsplot on remote server.

96891dd5f1b75402

@mattn
Copy link
Contributor Author

mattn commented Apr 12, 2018

I don't have Mac so I can't try jplot on iTerm2. How jplot works on white background?

@rs
Copy link
Owner

rs commented Apr 12, 2018

This is how it looks with a white background: screen shot 2018-04-11 at 7 36 25 pm

@mattn
Copy link
Contributor Author

mattn commented Apr 12, 2018

@saitoha go-sixel doesn't handle alpha colors. Can you figure out this problem?

@mattn
Copy link
Contributor Author

mattn commented Jul 3, 2018

Rebased from master branch. If I still have a change, could you please try this patch of go-sixel?

diff --git a/sixel.go b/sixel.go
index 8fdda6c..b5459f2 100644
--- a/sixel.go
+++ b/sixel.go
@@ -32,7 +32,7 @@ const (

 // Encode do encoding
 func (e *Encoder) Encode(img image.Image) error {
-       nc := 256 // (>= 2, 8bit, index 0 is reserved for transparent key color)
+       nc := 255 // (>= 2, 8bit, index 0 is reserved for transparent key color)
        width, height := img.Bounds().Dx(), img.Bounds().Dy()
        // make adaptive palette using median cut alogrithm
        q := median.Quantizer(nc - 1)

@saitoha
Copy link

saitoha commented Jul 3, 2018

@saitoha go-sixel doesn't handle alpha colors. Can you figure out this problem?

@mattn Traditional sixel format can represent transparent background(alpha=0).
It seems work fine on my xterm/mlterm.

pngsuite

Some terminal implementations can understand alpha-channeled palette. But go-sixel does not support it.
https://github.com/meh/cancer
http://nanno.dip.jp/softlib/man/rlogin/

@mattn
Copy link
Contributor Author

mattn commented Oct 15, 2018

I fixed problem that can't recognize terminal width/height. Also fixed white background issue.

@rs
Copy link
Owner

rs commented Oct 15, 2018

Sorry I think I wasn't clear. My question about osc package name was more: is sixel a subset of osc? If it's the case we can keep it in the osc package, otherwise we might want to think about a more generic name than osc. I prefer not to multiply the number of packages.

@mattn
Copy link
Contributor Author

mattn commented Oct 16, 2018

On my side, I'm sorry.

My question about osc package name was more: is sixel a subset of osc?

Sorry too. No. sixel is dsc. Could you please explain what do you like to change? Thanks.

@rs
Copy link
Owner

rs commented Oct 16, 2018

It’s more a brainstorm question. Can dsc be considered a subset or osc or they are two very different beats?

@mattn
Copy link
Contributor Author

mattn commented Oct 16, 2018

How about more abstract name like command, term or etc?

@rs
Copy link
Owner

rs commented Oct 16, 2018

term sounds good.

@rs
Copy link
Owner

rs commented Dec 31, 2018

I just tested this branch with xterm -ti vt340 and I get no output, the term stays blank. Any idea?

@rs
Copy link
Owner

rs commented Dec 31, 2018

Some good sixel tricks here: https://raw.githubusercontent.com/hackerb9/lsix/master/lsix

@mattn
Copy link
Contributor Author

mattn commented Dec 31, 2018

Which go version did you try?

@mattn
Copy link
Contributor Author

mattn commented Sep 18, 2019

Current build on master branch seems broken. This pull request include fixes for that.

@mattn
Copy link
Contributor Author

mattn commented Sep 13, 2021

@rs Do you have plan to merge this? If not, I'll maintenance this fork.

@rs rs merged commit bc6bc67 into rs:master Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants