fyne-io / fyne

Cross platform GUI toolkit in Go inspired by Material Design
https://fyne.io/
Other
25.13k stars 1.4k forks source link

Memory leak in fontMetrics cache on desktop driver #4010

Closed longkui-clown closed 1 month ago

longkui-clown commented 1 year ago

Checklist

Describe the bug

How to reproduce

Screenshots

No response

Example code


import ( "fmt" "net/http" _ "net/http/pprof" "strings" "time"

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

)

var logs = []string{} var maxLogTake = 50

func main() { go func() { http.ListenAndServe("0.0.0.0:10000", nil) }() app := app.New() win := app.NewWindow("Hello World!") logArea := widget.NewMultiLineEntry() logArea.SetMinRowsVisible(15)

content := container.NewVBox(
    container.NewHBox(
        widget.NewLabel("Click the button"),
        widget.NewButton("Log Test", func() {
            s_now := time.Now().Format("2006-01-02 15:04:05")
            for i := 0; i < 50; i++ {
                logs = append(logs, fmt.Sprintf("[Log](%v): %v", s_now, i))
            }

            logLen := len(logs)
            if logLen > maxLogTake {
                logs = append(logs[0:0], logs[logLen-maxLogTake:]...)
                logLen = maxLogTake
            }

            s_logs := strings.Join(logs, "\r\n")
            logArea.SetText(s_logs)
            logArea.CursorRow = logLen
            logArea.Refresh()
        }),
    ),
    logArea,
)

win.Resize(fyne.NewSize(450, 320))
win.SetContent(content)
win.ShowAndRun()

}



- click some times
![click1](https://github.com/fyne-io/fyne/assets/39942604/53a0597d-5bde-4a0e-b73f-309cb0b4dc94)

- click some times again
![click2](https://github.com/fyne-io/fyne/assets/39942604/f67684ad-be73-421f-91a6-9c4a90c63275)

- And can see the memory is increasing, I run again, and same 

### Fyne version

2.3.5

### Go compiler version

1.20.2

### Operating system and version

Windows 10

### Additional Information

_No response_
longkui-clown commented 1 year ago
andydotxyz commented 1 year ago

I believe this relates to text caches operating at a line level instead of per-glyph rendered. However you should be aware that everything drawn ok screen does get cached, so continually setting more text will increase memory used.

longkui-clown commented 1 year ago

yes, this design is perfect, while maybe someone don't know this design, and use it like me, will going to end up in a situation similar,I think there should have a switch function to control if use the cache, when use SetMinRowsVisible, it seem like no need to cache so many items

andydotxyz commented 1 year ago

Turning off texture caching means calculating and drawing text on every single GPU frame, it's a very bad idea.

I think what you want here is for Entry to be smarter to not have to draw all its content. At this time that's planned but not added. Realistically an editor widget for a log dump isn't ideal, a List of Labels would be a lot, lot faster as list contains the types of optimisation I mentioned.

Goltanju commented 1 year ago

Apologies for interrupting here but I actually use a multiline entry for a log listing and other listings on occasion as well. One of the big reasons I do this is because there's no way for a user to copy text from a label. For some cases, I'll make a big label at first and the user has to click a button (or menu item, etc.) to switch to edit mode so they can copy, which is a bit hacky and let's the user paste where I don't really want them to, but.

andydotxyz commented 1 year ago

You'll notice that in many apps a "copy button" alongside a commonly-copied line of text is a better user experience. Google, GitHub and others have started doing this for security codes and code snippets etc. Interacting with text to copy it out from an Entry isn't really the optimal user experience unless you are literally in a text editor in my opinion.

Goltanju commented 1 year ago

How would you implement a log viewer using Fyne? Copying the whole log usually isn't what a user wants. Copying a single line is probably a 90% solution, but a copy button for every line seems hacky and it doesn't handle the other 10% (made up percentages haha) where the user wants to grab several log lines.

Another example is a note taking app, think Evernote and the like. Here, 90% of the time the user will just be viewing notes and copying parts of them here and there as they need them elsewhere. 10% of the time they'll be editing. Having a read-only yet interactable view of a note would be huge here. Right now I show a read-only version, but they can't copy from it like they could on a web page, for example. They have to enter edit-mode just to copy, or I could leave it always in edit-mode which offers up new problems, haha.

Much of this pushes towards making these particular things webapps, but I really like Fyne! haha

andydotxyz commented 1 year ago

Disabled Entry is read only but selectable. Typically I would say a lot viewer copy a line would be ok - with export to file for the whole lot I guess.

At some point we will manage to optimise Entry to handle these huge texts, using similar techniques that have been polished in list/table/tree.

longkui-clown commented 1 year ago