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

Resizing List causes visible items to refresh instead of resize #4080

Closed
2 tasks done
kendellfab opened this issue Jul 21, 2023 · 16 comments
Closed
2 tasks done

Resizing List causes visible items to refresh instead of resize #4080

kendellfab opened this issue Jul 21, 2023 · 16 comments
Assignees
Labels
unverified A bug that has been reported but not verified

Comments

@kendellfab
Copy link

Checklist

  • I have searched the issue tracker for open issues that relate to the same problem, before opening a new one.
  • This issue only relates to a single bug. I will open new issues for any other problems.

Describe the bug

When an image is on one side of a split view, scrubbing the split view back and forth has a large delay as the image is being updated. This was working well on the v2.3.5 release, but appeared in the develop branch.

How to reproduce

  1. Create a window with a split
  2. One side of the split add a basic widget (i.e. a label)
  3. On the other side add an image (it might be size dependent on how big the impact is to performance)
  4. Scrub the split view left and right

Screenshots

FyneRegressionShort.mp4

Example code

package main

import (
"encoding/base64"
"log"

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

)

func main() {
regression := app.New()
window := regression.NewWindow("Regression")
window.Resize(fyne.NewSize(1200, 700))

leftLbl := widget.NewLabel("Left Side")

    // I can't put a large enough image into this text editor on github for an example, but I'm seeing this with images that are 600x400 and up, use one of those
bs, err := base64.StdEncoding.DecodeString(imageData)
if err != nil {
	log.Fatalln("error loading image data", err)
}

img := canvas.NewImageFromResource(fyne.NewStaticResource("example", bs))

split := container.NewHSplit(leftLbl, img)
split.SetOffset(0.25)

window.SetContent(split)
window.ShowAndRun()

}

const (
imageData = ``
)

Fyne version

develop

Go compiler version

1.20.6

Operating system and version

Arch Linux

Additional Information

No response

@kendellfab kendellfab added the unverified A bug that has been reported but not verified label Jul 21, 2023
@andydotxyz
Copy link
Member

Ah, maybe we have refreshed too deeply on a resize following changes.

Does this change sort things for you? (asking in case I can't get a suitable image locally...)

diff --git a/canvas/image.go b/canvas/image.go
index e22381fbd..ae0cf3fa5 100644
--- a/canvas/image.go
+++ b/canvas/image.go
@@ -180,7 +180,7 @@ func (i *Image) Resize(s fyne.Size) {
        }
 
        i.baseObject.Resize(s)
-       i.Refresh()
+       Refresh(i)
 }
 
 // NewImageFromFile creates a new image from a local file.

@kendellfab
Copy link
Author

I tried this update in the one off example that I posted above. It helped there. So I tried it in the app shown above, and it didn't hep as much as I'd expect. So I played around with the app, and noticed that there was that sluggishness even without showing the big image.

One thing I noticed, in the video attached, is the highlights for the list rows seem to flicker and cause some slow down as well.

FyneNoImage.mp4

@andydotxyz
Copy link
Member

One thing I noticed, in the video attached, is the highlights for the list rows seem to flicker and cause some slow down as well.

It's just a guess but looks like you have some pretty custom list items that have perhaps overridden the Tapped or Hover handling and it's going round in circles? Additionally it seems that resizing your list items is slow - perhaps something in your custom item widget could be impacting that?

@kendellfab
Copy link
Author

They are custom widgets, but they do not override any tapping or hovering methods. I can look into the code for the layout, but I didn't notice this issue until the latest development branch, which to me would indicate another performance issue in fyne somewhere.

@andydotxyz
Copy link
Member

The focus handling has changed in collections with 2.4 which could be why you didn't get this before...

@kendellfab
Copy link
Author

How then, would one go about addressing this change in performance in their application? As the update took a usable app and made it impossibly sluggish.

@andydotxyz
Copy link
Member

How then, would one go about addressing this change in performance in their application? As the update took a usable app and made it impossibly sluggish.

Sorry I was referring to the flicker of select / focus on the list.

As for performance I don't know - can you help us understand what is happening with a CPU profile report? None of our test apps are demonstrating this performance hit.

@kendellfab
Copy link
Author

I've run pprof.StartCPUProfile with the latest version of fyne and the development version. How would you like me to send those profiles?

@andydotxyz
Copy link
Member

I'm sorry I missed this message. Can you attach an SVG output of the report perhaps? I think GitHub is happy with that file type.

@kendellfab
Copy link
Author

Latest Build

weatherlight-fyne-v2 3 5

Develop

weatherlight-fyne-develop

@kendellfab
Copy link
Author

Yesterday I had some time to test this some more, and I tried it on my M2 Macbook air, and didn't notice any of the sluggishness. I could try it on windows if needed as well.

@andydotxyz
Copy link
Member

It would be good to know how things are with latest develop, most of the improvements are now landed.

@andydotxyz
Copy link
Member

From the CPU profiles it does look like it should be fixed from the change I added - it was continually re-loading the jpeg which is what I resolved.
Unless something further up the UI is calling Refresh on the image each time the Split is moved?

@kendellfab
Copy link
Author

I have been digging into this a bit more. As a quick aside, I have lists that are the first child widgets of two splits.

I took out all of my fancy layouts, and left just the images and the names. I then added loggers in my code that updates the images. And I've found that yes, the images are getting set a lot. But the trick is, this is coming from the UpdateTemplate function that is attached to the list. It appears as though, scrubbing the split is causing the UpdateTemplate functions to be called many times on each list that is being displayed.

Why then would the UpdateTemplate function be called on a split scrub?

@andydotxyz
Copy link
Member

That is strange. It would seem that listItem.Resize is calling Refresh - or somehow List is updating when it resizes.
I will update this issue with that as the main focus as we can fix that :)

@andydotxyz andydotxyz changed the title Image Performance Regression Resizing List causes visible items to refresh instead of resize Sep 3, 2023
@andydotxyz andydotxyz added this to the D fixes (v2.4.x) milestone Sep 3, 2023
@dweymouth dweymouth self-assigned this Aug 5, 2024
@dweymouth
Copy link
Contributor

Fixed on release branch for 2.5.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unverified A bug that has been reported but not verified
Projects
None yet
Development

No branches or pull requests

3 participants