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

Jump to bottom

x/exp/shiny/screen: Weird transition when uploading scaled texture in Ubuntu and macOS #36111

Open
manu-chroma opened this issue Dec 12, 2019 · 5 comments
Open

x/exp/shiny/screen: Weird transition when uploading scaled texture in Ubuntu and macOS #36111

manu-chroma opened this issue Dec 12, 2019 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@manu-chroma
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.9.7 darwin/amd64

Does this issue reproduce with the latest release?

NA

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/manvendra.s/go"
GORACE=""
GOROOT="/usr/local/opt/go@1.9/libexec"
GOTOOLDIR="/usr/local/opt/go@1.9/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/r2/drncpk212xxdw_zd5ns7nrrn09q2nk/T/go-build553917634=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

I am trying to write a CHIP-8 emulator in Go using golang.org/x/shiny/screen for displaying video buffer onto the screen.

The original resolution of the emulator screen is 32x64 pixels. So, I enlarge the graphical view of the emulator screen by scaling the existing buffer by some factor and then publishing it on the screen.

Here's the code snippet for the same:

case paint.Event:
            log.Print("Paint event, re-painting the buffer..")

            tex, _ := s.NewTexture(dim)
            defer tex.Release()

            tex.Upload(image.Point{}, drawBuff, drawBuff.Bounds())

            scaledDim := image.Rectangle{
                image.Point{0, 0},
                image.Point{EmuWidth * EmuScale, EmuHeight * EmuScale}}

            window.Scale(scaledDim, tex, drawBuff.Bounds(), draw.Over, &screen.DrawOptions{})
            window.Publish()

What did you expect to see?

On Windows:

https://i.stack.imgur.com/1iMZc.gif

What did you see instead?

On Mac and Ubuntu 19.10:

https://i.stack.imgur.com/cf2kf.gif
@gopherbot gopherbot added this to the Unreleased milestone Dec 12, 2019
@toothrot
Copy link
Contributor

/cc @nigeltao

@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 13, 2019
@nigeltao
Copy link
Contributor

Yeah, this looks like "nearest neighbor" vs "bilinear" scaling. To confirm that, on Ubuntu, you could changing the "bilinear" strings to "nearest" strings at:

https://github.com/golang/exp/blob/2f50522955873285d9bf17ec91e55aec8ae82edc/shiny/driver/x11driver/screen.go#L362

There's an existing TODO to add a DrawOptions field to control this:

https://github.com/golang/exp/blob/2f50522955873285d9bf17ec91e55aec8ae82edc/shiny/screen/screen.go#L353

Unfortunately, I don't have any spare time to work on this myself. Sorry.

@manu-chroma
Copy link
Author

manu-chroma commented Dec 14, 2019 •
edited

Hi @nigeltao, changing the string fixes the issue. 😄

I will be happy to pick this issue up, if you or anyone else can help out regarding where to make changes.

I see that https://github.com/golang/exp/blob/2f50522955873285d9bf17ec91e55aec8ae82edc/shiny/driver/x11driver/screen.go#L362 happens when creating a new texture whose interface method unfortunately does not take DrawOptions field.

and when Scaling the texture, this method is called hhttps://github.com/golang/exp/blob/master/shiny/driver/x11driver/texture.go#L126

Will we have to change the NewTexture interface to make this change?

Thanks!

@nigeltao
Copy link
Contributor

Yeah, the intent of the API isn't to select "nearest" vs "bilinear" at texture creation time (NewTexture), it's to select it at draw time (Draw and DrawOptions). Any given texture can be Draw'ed multiple times, with different DrawOptions each time.

So the fix isn't to change NewTexture. The fix is to make the Drawer methods look in the (possibly nil) *DrawOptions and set the driver-specific thing based on the (not implemented yet) DrawOptions.Scaler field.

For the x11driver, you could probably add a render.SetPictureFilter call next to the render.SetPictureTransform calls.

Supporting DrawOptions needs doing not just for the x11driver, but ideally for the other drivers too: OpenGL and Win32.

It's not going to be a trivial change, and I don't have a lot of spare time right now to provide detailed guidance. Sorry.

@gopherbot
Copy link

Change https://golang.org/cl/256937 mentions this issue: x/exp/shiny/screen: specify scaler filtering algorithm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@toothrot @nigeltao @gopherbot @manu-chroma