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

Moving canvas items (e.g. Images) does not cause canvas repaint #2205

Closed
pwiecz opened this issue Apr 28, 2021 · 6 comments
Closed

Moving canvas items (e.g. Images) does not cause canvas repaint #2205

pwiecz opened this issue Apr 28, 2021 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@pwiecz
Copy link
Contributor

pwiecz commented Apr 28, 2021

Describe the bug:

Calling Move on a canvas item such as canvas.Image does not cause canvas to automatically repaint.
Actually there does not seem to be a clean way to just cause canvas to repaint, without recreating the underlying canvas objects
(e.g. calling image.Refresh will also recreate the texture created from the image which may be costly).

To Reproduce:

Steps to reproduce the behaviour:

  1. Create a custom widget with a canvas.Image as one or several of its constituent objects
  2. Add a Dragged() method which moves the widget's images, but does not call Refresh on them.
  3. Run the app, and try to drag the image.
  4. Nothing happens, expected the images to move.

Example code:

package main

import (
	"image"
	"image/color"
	"image/draw"

	"fyne.io/fyne/v2"
	"fyne.io/fyne/v2/app"
	"fyne.io/fyne/v2/canvas"
	"fyne.io/fyne/v2/widget"
)

type testWidget struct {
	widget.BaseWidget
	background *canvas.Rectangle
	images     []*canvas.Image
	objects    []fyne.CanvasObject
}

func NewTestWidget() *testWidget {
	w := &testWidget{}
	w.ExtendBaseWidget(w)
	w.background = canvas.NewRectangle(color.White)
	img := image.NewNRGBA(image.Rect(0, 0, 256, 256))
	draw.Draw(img, img.Bounds(), &image.Uniform{color.Black}, image.ZP, draw.Src)

	w.objects = []fyne.CanvasObject{w.background}
	for ix := 0; ix < 16; ix++ {
		cImg := canvas.NewImageFromImage(img)
		cImg.Move(fyne.Position{float32((ix % 4) * 257), float32((ix / 4) * 257)})
		cImg.Resize(fyne.Size{256, 256})
		w.images = append(w.images, cImg)
		w.objects = append(w.objects, cImg)
	}
	return w
}
func (w *testWidget) CreateRenderer() fyne.WidgetRenderer { return w }
func (w *testWidget) Layout(size fyne.Size)               { w.background.Resize(size) }
func (w *testWidget) MinSize() fyne.Size                  { return fyne.Size{800, 600} }
func (w *testWidget) Refresh()                            {}
func (w *testWidget) Objects() []fyne.CanvasObject        { return w.objects }
func (w *testWidget) Destroy()                            {}
func (w *testWidget) Dragged(event *fyne.DragEvent) {
	dx, dy := event.Dragged.DX, event.Dragged.DY
	for _, img := range w.images {
		position := img.Position()
		img.Move(fyne.Position{position.X + dx, position.Y + dy})
		// img.Refresh() // uncommenting this line makes animation junky
	}
	// w.background.Refresh() // uncommenting this line seems to work, but seems hacky
}
func (w *testWidget) DragEnd() {}

func main() {
	a := app.New()
	w := a.NewWindow("Test dragging")
	w.SetContent(NewTestWidget())
	w.ShowAndRun()
}

-->

Device (please complete the following information):

  • OS: Linux
  • Version: 5.4.0
  • Go version: 1.16.2
  • Fyne version: 2.0.2
@pwiecz pwiecz added the bug Something isn't working label Apr 28, 2021
@andydotxyz
Copy link
Member

Thanks for this, I think perhaps we should add a flag to the canvas that repaint should be done after a Move call (Resize triggers other repaint events).

@andydotxyz andydotxyz added this to the Aberlour (2.1) milestone Apr 28, 2021
@fpabl0
Copy link
Member

fpabl0 commented Apr 28, 2021

Maybe this should be enough (after sharing code between the two drivers):

canvas/base.go:

// Repaint repaints the containing canvas.
func Repaint(obj fyne.CanvasObject) {
	if fyne.CurrentApp() == nil || fyne.CurrentApp().Driver() == nil {
		return
	}

	c := fyne.CurrentApp().Driver().CanvasForObject(obj)
	if c != nil {
		c.(interface{ SetDirty(bool) }).SetDirty(true)
	}
}

And then just call this in canvasobject.Move and in BaseWidget implementation (or maybe just in canvas objects 🤔)

@andydotxyz
Copy link
Member

I think if there were to be a canvas.Repaint it would be for a Canvas parameter not CanvasObject.
Then we could have the rest of the helper (looking up the canvas) as a private bit of code in the package.

@andydotxyz
Copy link
Member

This needs to be in 2.2 as we don't add API in fixes sorry for the confusion

@andydotxyz
Copy link
Member

Working on it now. Should not have been moved to a point release sadly as noted above due to new api requirement.

andydotxyz added a commit to andydotxyz/fyne that referenced this issue Apr 23, 2023
Move refresh to repaint where appropriate
And update on moves.

Fixes fyne-io#2205
@andydotxyz
Copy link
Member

Thanks to the suggestion by @fpabl0 there is a version of this fix without a new API, so trying to get it in to next point release.

andydotxyz added a commit that referenced this issue Apr 28, 2023
Move refresh to repaint where appropriate
And update on moves.

Fixes #2205
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants