AllenDang / giu

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

[bug] ImageWithRgba and ImageWithFile internal state incorrectly indexed (incorrect internal keys) #509

Open rasteric opened 2 years ago

rasteric commented 2 years ago

What happend?

ImageWithRgba and ImageWithFile maintain internal state in Context. However, this information is not updated correctly when new image widgets with new images are created in the render loop but the total number of widgets remains the same. The symptom of this problem is that if a window displays n widgets, and one of them is an image widget and a new image widget with another image is created, then the GUI sometimes displays the old image.

The reason seems to be that the widget's internal id is created with GenAutoID but this function only uses a string based on e.g. "ImageWithRgba" plus a simple widget counter:

// GenAutoID automatically generates fidget's id.
func GenAutoID(id string) string {
    return fmt.Sprintf("%s##%d", id, Context.GetWidgetIndex())
}

This does not seem to be fine-grained enough to store image textures in the context when the image changes (calling e.g. ImageWithRgba with different images in the render loop) but the total number of widgets remains constant. Problems with other widgets might also occur if they use the ID to store state in the context.

Code example

main.go ```golang package main import ( "image" "image/draw" "image/color" g "github.com/AllenDang/giu" ) var img1 draw.Image var img2 draw.Image var toggle bool var splitPos float32 func main() { img1 = image.NewRGBA(image.Rect(0, 0, 400, 400)) blue := color.RGBA{0, 0, 255, 255} draw.Draw(img1, img1.Bounds(), &image.Uniform{blue}, image.ZP, draw.Src) img2 = image.NewRGBA(image.Rect(0, 0, 400, 400)) red := color.RGBA{255, 0, 0, 255} draw.Draw(img2, img2.Bounds(), &image.Uniform{red}, image.ZP, draw.Src) splitPos = 100.0 main:=g.NewMasterWindow("test", 800, 800,0) main.Run(loop) } func loop() { var img image.Image if toggle { img = img2 } else { img = img1 } g.SingleWindow().Layout( g.SplitLayout(g.DirectionHorizontal, splitPos, g.Button("toggle").OnClick(func() { toggle = !toggle g.Update() }), g.ImageWithRgba(img).Size(400,400), ), ) } ```

To Reproduce

  1. Run the demo
  2. Press toggle -> nothing happens, the blue square is continued to be displayed
  3. Move the slider all the way to the right and back -> now the red square is displayed

The reason is presumably that the internal context state is only changed when the widget count changes, which is caused when the slider is all the way to the right.

Version

(latest)

OS

Linux Mint 20.3 Cinnamom

gucio321 commented 2 years ago

well, I believe, that this particular case (of ImageWidget) could be solved by increasing complexity of widget's ID base e.g. generate some type of "checksum" for RGBA data or something like that.

id:   GenAutoID("ImageWithRgba" /* <- put here something more complex */) 

what do you think about it @AllenDang ?

AllenDang commented 2 years ago

@gucio321 Generate a checksum every frame is way too slow... @rasteric I think you could set a new ID to image widget after the texture is changed

rasteric commented 2 years ago

Using a checksum shouldn't be necessary unless this is supposed to be safe for direct image manipulation, too. Shouldn't it just involve a pointer comparison otherwise?

I agree setting a new ID is a reasonable way to solve the issue, after all the programmer knows when the image is changed. But is there a public API for it?

AllenDang commented 2 years ago

@rasteric I just added ID setter to ImageWithRgbaWidget and ImageWithFileWidget update to newest master branch to get it.

gucio321 commented 2 years ago

@AllenDang i'd add a comment over ImageWidget pointing to this issue as well

And perhaps over GenAutoID too

0xkalle commented 9 months ago

Could it happen within this bug, that texture is swapped with a text atlas? Sometime my picture swaps to this: grafik And I don't understand why yet.

Should have been that picture: grafik

The texture pointer variable is never accessed in my code except from loading and in the g.Image()...

It somehow sounds similar to this issue here using a g.ImageFromRGBA did not help even with ID(...) I load the image like in the image example.

Does somebody has an idea, what I am doing wrong?

gucio321 commented 9 months ago

it looks like something wrong with internal imgui texture handling... hard to say without code.

0xkalle commented 9 months ago

I'll try to get a minimal code example together as soon as I have time. :)