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

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

Open
rasteric opened this issue May 17, 2022 · 8 comments
Labels
bug Something isn't working documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@rasteric
Copy link

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
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

@rasteric rasteric added the bug Something isn't working label May 17, 2022
@gucio321
Copy link
Collaborator

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
Copy link
Owner

@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
Copy link
Author

rasteric commented Jun 2, 2022

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
Copy link
Owner

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

@gucio321
Copy link
Collaborator

gucio321 commented Jul 12, 2022

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

And perhaps over GenAutoID too

@gucio321 gucio321 added documentation Improvements or additions to documentation good first issue Good for newcomers labels May 9, 2023
@0xkalle
Copy link

0xkalle commented Feb 14, 2024

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
Copy link
Collaborator

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

@0xkalle
Copy link

0xkalle commented Feb 14, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants