AllenDang / giu

Cross platform rapid GUI framework for golang based on Dear ImGui.
MIT License
2.34k stars 134 forks source link

[bug] crashes when changing texture on a canvas (DeleteTexture / segmentation violation) #879

Open ioxenus opened 1 week ago

ioxenus commented 1 week ago

What happend?

This is possibly related to #854 and #819, but the issue still persists for me even with the latest commits:

I made a custom widget that displays a map with a bunch of countries. When I click some country, it gets highlighted and it shows in the UI. I made this using an image mask that has the countries colored with different colors, and when I click on the widget - it gets the pixel color of the clicked country at the (X, Y) and then generates a new image with the highlighted area.

After changing the displayed image a couple of times, the app crashes:

Log & stacktrace ``` GetCurrentTexture(): NewTextureFromRgba id={4} GetCurrentTexture(): done, returning new texture GetCurrentTexture(): 1) cloning image GetCurrentTexture(): 2) creating mask GetCurrentTexture(): 3) converting to draw.Image GetCurrentTexture(): 4) drawing mask GetCurrentTexture(): 5) converting back to image.Image GetCurrentTexture(): 6) mask applied, generating texture GetCurrentTexture(): NewTextureFromRgba id={5} GetCurrentTexture(): done, returning new texture GetCurrentTexture(): 1) cloning image GetCurrentTexture(): 2) creating mask backend/texture.go: Texture.release() -> DeleteTexture({4}) SIGSEGV: segmentation violation PC=0x1f5bbc8f8 m=9 sigcode=2 addr=0x30 signal arrived during cgo execution goroutine 34 gp=0x14000186380 m=9 mp=0x1400008c808 [syscall]: runtime.cgocall(0x1006eca48, 0x14000068ca8) /opt/homebrew/Cellar/go/1.23.2/libexec/src/runtime/cgocall.go:167 +0x44 fp=0x14000068c70 sp=0x14000068c30 pc=0x1005bba44 github.com/AllenDang/cimgui-go/backend/glfwbackend._Cfunc_igDeleteTexture(0x4) _cgo_gotypes.go:388 +0x30 fp=0x14000068ca0 sp=0x14000068c70 pc=0x1006350a0 github.com/AllenDang/cimgui-go/backend/glfwbackend.(*GLFWBackend).DeleteTexture.func1({0x1400023c0d0?}) ~/go/pkg/mod/github.com/!allen!dang/cimgui-go@v1.0.3-0.20241007063847-1a15ba4f9967/backend/glfwbackend/glfw_backend.go:375 +0x40 fp=0x14000068ce0 sp=0x14000068ca0 pc=0x100635ee0 github.com/AllenDang/cimgui-go/backend/glfwbackend.(*GLFWBackend).DeleteTexture(0x100a9b348?, {0x1400019c018?}) ~/go/pkg/mod/github.com/!allen!dang/cimgui-go@v1.0.3-0.20241007063847-1a15ba4f9967/backend/glfwbackend/glfw_backend.go:375 +0x20 fp=0x14000068d00 sp=0x14000068ce0 pc=0x100635e70 github.com/AllenDang/cimgui-go/backend.(*Texture).release(0x1400001c2a0) ~/go/pkg/mod/github.com/!allen!dang/cimgui-go@v1.0.3-0.20241007063847-1a15ba4f9967/backend/texture.go:39 +0x98 fp=0x14000068d60 sp=0x14000068d00 pc=0x1006347b8 runtime.call16(0x0, 0x100a9a198, 0x140005c4000, 0x10, 0x10, 0x10, 0x14000068e00) /opt/homebrew/Cellar/go/1.23.2/libexec/src/runtime/asm_arm64.s:504 +0x78 fp=0x14000068d80 sp=0x14000068d60 pc=0x1005c6838 runtime.runfinq() /opt/homebrew/Cellar/go/1.23.2/libexec/src/runtime/mfinal.go:255 +0x3e4 fp=0x14000068fd0 sp=0x14000068d80 pc=0x10056bea4 runtime.goexit({}) /opt/homebrew/Cellar/go/1.23.2/libexec/src/runtime/asm_arm64.s:1223 +0x4 fp=0x14000068fd0 sp=0x14000068fd0 pc=0x1005c8834 created by runtime.createfing in goroutine 1 /opt/homebrew/Cellar/go/1.23.2/libexec/src/runtime/mfinal.go:163 +0x80 ```

I tried to debug/refactor it with no success; didn't have this issue on giu v0.7.0.

Code example

main.go ```golang package main import ( "fmt" "image" "image/color" "image/draw" "os" "slices" "strings" "github.com/AllenDang/cimgui-go/imgui" g "github.com/AllenDang/giu" ) var ( europe *MapWidget ) type MapWidget struct { img_w float32 img_h float32 img_size image.Point current_texture *g.Texture should_refresh bool i_texture *g.Texture i_image *image.Image i_highlighted *image.RGBA i_area *image.RGBA palette map[color.Color]int palette_int map[int]color.Color selected_area_ids []int } func NewMapWidget() *MapWidget { return &MapWidget{} } func (m *MapWidget) Init() { fmt.Printf("Init()\n") m.palette = map[color.Color]int{} m.palette_int = map[int]color.Color{} m.img_w = float32(512) m.img_h = float32(512) m.img_size = image.Pt(512, 512) m.selected_area_ids = []int{} m.should_refresh = true m.LoadPalette() m.LoadImage("") m.LoadImage("highlighted") m.LoadImage("areas") } func (m *MapWidget) LoadPalette() { img_palette, _ := g.LoadImage("images/palette.png") for x := range 24 { col := img_palette.At(x, 0) m.palette[col] = x + 1 m.palette_int[x+1] = color.RGBAModel.Convert(col) } fmt.Println() } func (m *MapWidget) LoadImage(suffix string) { path := fmt.Sprintf("images/map.png") if suffix == "" { img, _ := open_image_file(path) m.i_image = &img fmt.Printf("i_image loaded: ptr=%v\n", m.i_image) } else if suffix == "highlighted" { path = strings.Replace(path, ".png", "_highlighted.png", 1) img, _ := open_image_file(path) m.i_highlighted = g.ImageToRgba(img) fmt.Printf("i_highlighted loaded: ptr=%v\n", m.i_image) } else if suffix == "areas" { path = strings.Replace(path, ".png", "_areas.png", 1) img, _ := open_image_file(path) m.i_area = g.ImageToRgba(img) fmt.Printf("i_area loaded: ptr=%v\n", m.i_image) } } func (m *MapWidget) LoadTexture() { path := fmt.Sprintf("images/map.png") i, _ := g.LoadImage(path) g.NewTextureFromRgba(i, func(tex *g.Texture) { fmt.Printf("LoadTexture(): NewTextureFromRgba id=%v\n", tex.ID()) m.i_texture = tex m.current_texture = tex }) } func (m *MapWidget) ToggleSelectAreaID(area_id int) { if area_id == 0 { return } if slices.Contains(m.selected_area_ids, area_id) { m.selected_area_ids = slices.DeleteFunc(m.selected_area_ids, func(e int) bool { return e == area_id }) } else { m.selected_area_ids = append(m.selected_area_ids, area_id) } m.should_refresh = true } func (m *MapWidget) clonePix(b []uint8) []byte { // https://groups.google.com/g/golang-nuts/c/pic3Nya7DRg c := make([]uint8, len(b)) copy(c, b) return c } func (m *MapWidget) CloneImage(src image.Image) draw.Image { // https://groups.google.com/g/golang-nuts/c/pic3Nya7DRg switch s := src.(type) { case *image.Alpha: clone := *s clone.Pix = m.clonePix(s.Pix) return &clone case *image.Alpha16: clone := *s clone.Pix = m.clonePix(s.Pix) return &clone case *image.Gray: clone := *s clone.Pix = m.clonePix(s.Pix) return &clone case *image.Gray16: clone := *s clone.Pix = m.clonePix(s.Pix) return &clone case *image.NRGBA: clone := *s clone.Pix = m.clonePix(s.Pix) return &clone case *image.NRGBA64: clone := *s clone.Pix = m.clonePix(s.Pix) return &clone case *image.RGBA: clone := *s clone.Pix = m.clonePix(s.Pix) return &clone case *image.RGBA64: clone := *s clone.Pix = m.clonePix(s.Pix) return &clone } return nil } func (m *MapWidget) CreateMask(img image.Image) image.Image { black := color.RGBA{0, 0, 0, 0} white := color.RGBA{255, 0, 255, 255} var cols []color.Color for _, area_id := range m.selected_area_ids { cols = append(cols, m.palette_int[int(area_id)]) } b := img.Bounds() img_new := image.NewRGBA(b) for y := 0; y < b.Max.Y; y++ { for x := 0; x < b.Max.X; x++ { px_col := img.At(x, y) px_col_rgba := color.RGBAModel.Convert(px_col) if slices.Contains(cols, px_col_rgba) { img_new.Set(x, y, white) } else { img_new.Set(x, y, black) } } } return img_new } func (m *MapWidget) GetCurrentTexture() *g.Texture { if m.should_refresh { fmt.Printf("GetCurrentTexture(): 1) cloning image\n") img := m.CloneImage(*m.i_image) img_highlighted := m.i_highlighted img_area := m.i_area fmt.Printf("GetCurrentTexture(): 2) creating mask\n") img_mask := m.CreateMask(img_area) fmt.Printf("GetCurrentTexture(): 3) converting to draw.Image\n") img_final, _ := img.(draw.Image) fmt.Printf("GetCurrentTexture(): 4) drawing mask\n") draw.DrawMask(img_final, img.Bounds(), img_highlighted, image.Point{0, 0}, img_mask, img_mask.Bounds().Min, draw.Over) fmt.Printf("GetCurrentTexture(): 5) converting back to image.Image\n") img_final_image := img_final.(image.Image) fmt.Printf("GetCurrentTexture(): 6) mask applied, generating texture\n") g.NewTextureFromRgba(img_final_image, func(tex *g.Texture) { fmt.Printf("GetCurrentTexture(): NewTextureFromRgba id=%v\n", tex.ID()) m.current_texture = tex }) m.should_refresh = false fmt.Printf("GetCurrentTexture(): done, returning new texture\n") } return m.current_texture } func (m *MapWidget) Widget() *g.CustomWidget { return g.Custom(func() { canvas := g.GetCanvas() start_pos := g.GetCursorScreenPos() end_pos := start_pos.Add(m.img_size) id_str := g.GenAutoID("canvas_invisible_button").String() // fmt.Printf("[Widget()] id=%s\n", id_str) imgui.InvisibleButton(id_str, imgui.Vec2{X: float32(m.img_w), Y: float32(m.img_h)}) if m.i_texture == nil { fmt.Printf("Widget(): loading texture\n") m.LoadTexture() } else { // fmt.Printf("Widget(): getting current texture\n") cur_tex := m.GetCurrentTexture() // fmt.Printf("Widget(): cur_tex=%v, adding image to canvas\n", cur_tex) canvas.AddImage(cur_tex, start_pos, end_pos) } if g.IsMouseClicked(g.MouseButtonLeft) && g.IsWindowFocused(0) { mouse_pos := g.GetMousePos() rel_mouse_pos := mouse_pos.Sub(start_pos) if rel_mouse_pos.X >= 0 && rel_mouse_pos.X < m.img_size.X && rel_mouse_pos.Y >= 0 && rel_mouse_pos.Y < m.img_size.Y { mask_color := m.i_area.At(rel_mouse_pos.X, rel_mouse_pos.Y) area_id := m.palette[mask_color] m.ToggleSelectAreaID(area_id) } } }) } func loop() { map_thumbnail := europe.Widget() g.SingleWindow().Layout( g.Label("dummy label"), map_thumbnail, ) } func open_image_file(filepath string) (image.Image, error) { f, err := os.Open(filepath) if err != nil { fmt.Printf("open_image_file open err: %v\n", err) return nil, err } defer f.Close() img, _, err := image.Decode(f) if err != nil { fmt.Printf("open_image_file decode err: %v\n", err) return nil, err } return img, nil } func main() { europe = NewMapWidget() europe.Init() wnd := g.NewMasterWindow("Canvas", 600, 600, 0) wnd.Run(loop) } ```

To Reproduce

Put these images in the "images" directory (relative to main.go):

map_areas.png: https://github.com/user-attachments/assets/a5c3b998-aaff-4257-9fcc-ca59b8517b26 map.png: https://github.com/user-attachments/assets/dd4aacb1-785c-416c-bbcd-18a8f155997c map_highlighted.png: https://github.com/user-attachments/assets/e8b974fe-2789-446d-983d-dc23e585ff57 palette.png https://github.com/user-attachments/assets/653203e3-d09a-4d47-817b-bd7a9a22e297

Run the main.go and try clicking on a several countries.

Version

master

OS

darwin / arm64 (macOS 14.6.1)

ioxenus commented 1 week ago

I'm sorry, I seem to have fixed it.

Just noticed the new examples (asyncimage.go specifically) that use ReflectiveBoundTexture, so I rewrote my code by replacing g.Texture with g.ReflectiveBoundTexture.
(also, g.NewTextureFromRgba() -> rb_texture.SetSurfaceFromRGBA())
(and now I'm using rb_texture.Texture()to put it on the canvas).

cjbrigato commented 1 week ago

@ioxenus if you use ReflectiveBoundTexture please use master branch, not 0.9.0 release as it contains important fixe to avoid double freeing texture against glfw (see https://github.com/AllenDang/giu/commit/1f8aaafc321c69e8f5515ea6db7b93c6df453490#diff-7771525abe6cc9f582453f4e0b2c6ad87be391fd8b3cb6743c9a5d04d5be90b2)

ioxenus commented 4 days ago

Yeah, I use the master branch, thanks!

After fixing my own custom widget, I noticed that it still crashes. Turned out I was still using the g.ImageWithRgba widget somewhere else. Managed to fix that, too, by adding ReflectiveBoundTexture (with SetSurfaceFromRGBA) and using tex.ToImageWidget() instead of g.ImageWithRgba()

before:

var img_icon image.Image

// load image from file (skipping error handling just for the sake of readability)
f, _ := embed_fs.Open("icon.png")
defer f.Close()
img_icon, _, _ := image.Decode(f)

// somewhere in window layout (added some context)
cell := g.Style().SetStyle(g.StyleVarItemSpacing, 4, 4).To(
    g.ImageWithRgba(img_icon).Size(20, 20),  // was working in giu v0.7
)
table_row := g.TreeTableRow("foobar", cell)
table := g.TreeTable().Rows(...) // etc

The example above crashes after a while when the TreeTable gets updated (some rows are added/removed). The stacktrace was the same as above - something about releasing/deleting the texture.

after:

var img_icon *image.RGBA
var tex_icon = &g.ReflectiveBoundTexture{}

// load image from file
f, _ := embed_fs.Open("icon.png")
defer f.Close()
img, _, _ := image.Decode(f)
img_icon = g.ImageToRgba(img)

// initialize texture
tex_icon.SetSurfaceFromRGBA(img_icon, false)

// somewhere in window layout (skipped context)
tex_icon.ToImageWidget().Size(20, 20)

The code above seems to work fine with the current master branch of giu (f64eda1) & cimgui-go (1a15ba4).

I have no idea if this issue should stay opened since I found the solution, but it took me some struggling to pinpoint the issue, and, I guess, it may break compatibility after upgrading from giu v0.7 to a newer version of giu/cimgui-go.

I'm also pretty new with Go, so it could be that I was doing (and maybe still doing) something wrong. If that's the case - would really appreciate the feedback!

cjbrigato commented 2 days ago

@ioxenus we'll take a look at this since ImageWithRgba was thought to be (at last) fixed.

Aside from that, a quick advice: you do not need to do the image.Decode and g.ImageToRgba if you use g.ReflectiveBoundTexture as SetSurfaceFromFsFile takes a File object :)