veandco / go-sdl2

SDL2 binding for Go
https://godoc.org/github.com/veandco/go-sdl2
BSD 3-Clause "New" or "Revised" License
2.18k stars 219 forks source link

Invalid color converting in Surface.Set() #424

Open POPSuL opened 4 years ago

POPSuL commented 4 years ago

Example code for drawing RED square:

package main

import (
    "github.com/veandco/go-sdl2/sdl"
    "image/color"
    "os"
)

const width, height = 100, 100

func main() {
    if err := sdl.Init(sdl.INIT_EVERYTHING); err != nil {
        panic(err)
    }
    window, err := sdl.CreateWindow(
        "color bug",
        sdl.WINDOWPOS_UNDEFINED,
        sdl.WINDOWPOS_UNDEFINED,
        width,
        height,
        sdl.WINDOW_SHOWN)
    if err != nil {
        panic(err)
    }
    surface, err := window.GetSurface()
    if err != nil {
        panic(err)
    }

    // fill background with black color
    rect := sdl.Rect{W: width, H: height}
    surface.FillRect(&rect, 0xff000000)

    // draw RED square
    for x := width / 4; x < width - (width / 4); x++ {
        for y := height / 4; y < height - (height / 4); y++ {
            surface.Set(x, y, color.RGBA{
                R: 0xff,
                G: 0,
                B: 0,
                A: 0xff,
            })
        }
    }

    // flush our square
    window.UpdateSurface()

    for {
        for event := sdl.PollEvent(); event != nil; event = sdl.PollEvent() {
            if event.GetType() == sdl.QUIT {
                os.Exit(0)
            }
        }
    }
}

Problem: Windows 10 draws RED square, but in Linux (Ubuntu, KDE) that code draws BLUE square. Due debugging I found difference between pixel format in surface (windows have RGB888 but linux have RGBA8888). If we look at the code of Surface.Set() we can found 2 branches:

    case PIXELFORMAT_ARGB8888:
        col := surface.ColorModel().Convert(c).(color.RGBA)
        pix[i+0] = col.R
        pix[i+1] = col.G
        pix[i+2] = col.B
        pix[i+3] = col.A

And

    case PIXELFORMAT_RGB24, PIXELFORMAT_RGB888:
        col := surface.ColorModel().Convert(c).(color.RGBA)
        pix[i+0] = col.B
        pix[i+1] = col.G
        pix[i+2] = col.R

First branch executes on linux and not working correctly. Second branch executes on windows and work correctly. But, first should have "SAME" logic (+alpha), but in first case we have RGBA to RGBA conversion and it possible not correctly for SDL2 and we have RGBA to BRG which possible correctly.

veeableful commented 4 years ago

Hi @POPSuL, I'll have a detailed look at it sometime today! I will comment again once I know what causes the issue.

POPSuL commented 4 years ago

@veeableful thanks! I'll wait. If additional information needed - I can inform.

Btw, I force set pixel format via

surface.Format.Format = sdl.PIXELFORMAT_RGB888

(before calling Surface.Set()) and now that sample works correctly, and in linux looks like in windows (both OS draws RED square).

veeableful commented 4 years ago

Hi @POPSuL, it should have been fixed in the master branch now!

dev-abir commented 4 years ago

This example from @veeableful

Though the following code is not working. It is showing a blue rectangle, on GNU/Linux, I haven't tested it on Windows.

package main

import (
    "github.com/veandco/go-sdl2/sdl"
    "golang.org/x/image/colornames"
)

func main() {
    if err := sdl.Init(sdl.INIT_EVERYTHING); err != nil {
        panic(err)
    }
    defer sdl.Quit()

    window, err := sdl.CreateWindow("test", sdl.WINDOWPOS_UNDEFINED, sdl.WINDOWPOS_UNDEFINED,
        800, 600, sdl.WINDOW_SHOWN)
    if err != nil {
        panic(err)
    }
    defer window.Destroy()

    surface, err := window.GetSurface()
    if err != nil {
        panic(err)
    }
    surface.FillRect(nil, 0)

    rect := sdl.Rect{0, 0, 200, 200}
    surface.FillRect(&rect, sdl.Color(colornames.Red).Uint32())
    window.UpdateSurface()

    running := true
    for running {
        for event := sdl.PollEvent(); event != nil; event = sdl.PollEvent() {
            switch event.(type) {
            case *sdl.QuitEvent:
                println("Quit")
                running = false
                break
            }
        }
    }
}
veeableful commented 4 years ago

Hi @dev-abir! Unfortunately, the default pixel format used by the window surface is RGB888 which is incompatible with sdl.Color (RGBA8888).

You can work around this by creating another surface with matching pixel format and then blit it on to the window surface, like this:

...

    surface, err := window.GetSurface()
    if err != nil {
        panic(err)
    }
    surface.FillRect(nil, 0)

    rgbaSurface, err := sdl.CreateRGBSurfaceWithFormat(0, surface.W, surface.H, 32, sdl.PIXELFORMAT_RGBA8888)
    if err != nil {
        panic(err)
    }

    rect := sdl.Rect{0, 0, 200, 200}
    c := sdl.Color(colornames.Red).Uint32()
    rgbaSurface.FillRect(&rect, uint32(c))
    rgbaSurface.Blit(nil, surface, nil)
    window.UpdateSurface()

...
dev-abir commented 4 years ago

Hi @dev-abir! Unfortunately, the default pixel format used by the window surface is RGB888 which is incompatible with sdl.Color (RGBA8888).

You can work around this by creating another surface with matching pixel format and then blit it on to the window surface, like this:

...

  surface, err := window.GetSurface()
  if err != nil {
      panic(err)
  }
  surface.FillRect(nil, 0)

  rgbaSurface, err := sdl.CreateRGBSurfaceWithFormat(0, surface.W, surface.H, 32, sdl.PIXELFORMAT_RGBA8888)
  if err != nil {
      panic(err)
  }

  rect := sdl.Rect{0, 0, 200, 200}
  c := sdl.Color(colornames.Red).Uint32()
  rgbaSurface.FillRect(&rect, uint32(c))
  rgbaSurface.Blit(nil, surface, nil)
  window.UpdateSurface()

...

Okay thanks