golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.93k stars 17.66k forks source link

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

Open manu-chroma opened 4 years ago

manu-chroma commented 4 years ago

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

toothrot commented 4 years ago

/cc @nigeltao

nigeltao commented 4 years ago

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 commented 4 years ago

Hi @nigeltao, changing the string fixes the issue. :smile:

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 commented 4 years ago

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 commented 4 years ago

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