lxn / walk

A Windows GUI toolkit for the Go Programming Language
Other
6.84k stars 884 forks source link

there's problem about Children().Clear(),whether it can be optimized #593

Closed thomasbrook closed 5 years ago

thomasbrook commented 5 years ago

At first, a control of type scrollview has six child control of type compose. the child control will be removed, when i click button named create.Then,i call Children().Clear(), new child control will be add to parent control.

image image

image

Two child controls are not removed. Four child controls are not dispose.

package main

import (
    "fmt"

    "github.com/lxn/walk"
    . "github.com/lxn/walk/declarative"
)

func main() {
    mw := new(MyMainWindow)
    m := MainWindowInit(mw)
    if _, err := m.Run(); err != nil {
        fmt.Println(err)
    }
}

type MyMainWindow struct {
    *walk.MainWindow
    sv *walk.ScrollView
}

func MainWindowInit(mw *MyMainWindow) *MainWindow {

    m := &MainWindow{
        AssignTo:   &mw.MainWindow,
        Title:      "MainWindow",
        Background: SolidColorBrush{Color: walk.RGB(0x5F, 0x69, 0x8E)},
        Layout:     VBox{},
        Size:       Size{550, 340},
        Children: []Widget{
            PushButton{
                Text: "create",
                OnClicked: func() {
                    childlist := mw.sv.Children()
                    if childlist != nil {
                        childlist.Clear()
                        //                      for i := childlist.Len() - 1; i >= 0; i-- {
                        //                          ch := childlist.At(i)
                        //                          ch.SetParent(nil)
                        //                          ch.Dispose()
                        //                      }
                        fmt.Println("clear ok!")
                    }
                    loadChild(mw.sv)
                    return
                },
            },
            ScrollView{
                AssignTo: &mw.sv,
                Layout: Flow{
                    MarginsZero: true,
                    Alignment:   AlignHNearVNear,
                },
                Children: []Widget{
                    Composite{
                        Layout:     VBox{},
                        Background: SolidColorBrush{Color: walk.RGB(0x00, 0x8B, 0x8B)},
                        Children: []Widget{
                            Label{
                                Alignment: AlignHNearVCenter,
                                Font:      Font{PointSize: 11},
                                Text:      "hello walk ",
                                Visible:   true,
                                TextColor: walk.RGB(0xFF, 0xFF, 0xFF),
                            },
                        },
                    },
                    Composite{
                        Layout:     VBox{},
                        Background: SolidColorBrush{Color: walk.RGB(0x00, 0x8B, 0x8B)},
                        Children: []Widget{
                            Label{
                                Alignment: AlignHNearVCenter,
                                Font:      Font{PointSize: 11},
                                Text:      "I am fine. ",
                                Visible:   true,
                                TextColor: walk.RGB(0xFF, 0xFF, 0xFF),
                            },
                        },
                    },
                    Composite{
                        Layout:     VBox{},
                        Background: SolidColorBrush{Color: walk.RGB(0x00, 0x8B, 0x8B)},
                        Children: []Widget{
                            Label{
                                Alignment: AlignHNearVCenter,
                                Font:      Font{PointSize: 11},
                                Text:      "I am fine. ",
                                Visible:   true,
                                TextColor: walk.RGB(0xFF, 0xFF, 0xFF),
                            },
                        },
                    },
                    Composite{
                        Layout:     VBox{},
                        Background: SolidColorBrush{Color: walk.RGB(0x00, 0x8B, 0x8B)},
                        Children: []Widget{
                            Label{
                                Alignment: AlignHNearVCenter,
                                Font:      Font{PointSize: 11},
                                Text:      "I am fine. ",
                                Visible:   true,
                                TextColor: walk.RGB(0xFF, 0xFF, 0xFF),
                            },
                        },
                    },
                    Composite{
                        Layout:     VBox{},
                        Background: SolidColorBrush{Color: walk.RGB(0x00, 0x8B, 0x8B)},
                        Children: []Widget{
                            Label{
                                Alignment: AlignHNearVCenter,
                                Font:      Font{PointSize: 11},
                                Text:      "I am fine. ",
                                Visible:   true,
                                TextColor: walk.RGB(0xFF, 0xFF, 0xFF),
                            },
                        },
                    },
                    Composite{
                        Layout:     VBox{},
                        Background: SolidColorBrush{Color: walk.RGB(0x00, 0x8B, 0x8B)},
                        Children: []Widget{
                            Label{
                                Alignment: AlignHNearVCenter,
                                Font:      Font{PointSize: 11},
                                Text:      "I am fine. ",
                                Visible:   true,
                                TextColor: walk.RGB(0xFF, 0xFF, 0xFF),
                            },
                        },
                    },
                },
            },
        },
    }
    return m
}

func loadChild(parent walk.Container) error {
    for i := 0; i < 8; i++ {
        w := Composite{
            Layout:     VBox{},
            Background: SolidColorBrush{Color: walk.RGB(0x00, 0x8B, 0x8B)},
            Children: []Widget{
                Composite{
                    Layout: VBox{},
                    Children: []Widget{

                        Label{
                            Alignment: AlignHNearVCenter,
                            Font:      Font{PointSize: 11},
                            Text:      "smile",
                            Visible:   true,
                            TextColor: walk.RGB(0xFF, 0xFF, 0xFF),
                        },
                    },
                },
            },
        }

        err := w.Create(NewBuilder(parent))
        if err != nil {
            panic(err)
        }
    }

    return nil
}
lxn commented 5 years ago

You should change your OnClicked handler to something like this:

...
    OnClicked: func() {
        childlist := mw.sv.Children()
        if childlist != nil {
            mw.sv.SetSuspended(true)
            defer mw.sv.SetSuspended(false)

            for i := childlist.Len() - 1; i >= 0; i-- {
                childlist.At(i).Dispose()
            }
            fmt.Println("clear ok!")
        }
        loadChild(mw.sv)
        return
    },
...

WidgetList.Clear only removes all widgets from a container, but does not dispose of them. Calling Dispose on a widget on the other hand does both.

For best results you should always wrap operations that would trigger multiple layout updates in parent.SetSuspended(true); defer parent.SetSuspended(false).

thomasbrook commented 5 years ago

In this way, I did not think of a specific scenario. But,WidgetList.Clear is not remove all widgets from a container.It may be a bug. As you loop through the list in the method onClearingWidgets of the file container.go,you also remove the control from the list in the method SetParent.

I'm not sure if I'm right.

lxn commented 5 years ago

Clearing children should be fixed by https://github.com/lxn/walk/commit/65fec213208b35277f369146b05d233ad1de6504.

thomasbrook commented 5 years ago

After use parent.SetSuspended, usage experience is very fine.thanks.