From a50df3d7b4f6bfd93893343c5640873f811b1224 Mon Sep 17 00:00:00 2001 From: levovix Date: Fri, 6 Dec 2024 21:47:15 +0300 Subject: [PATCH] fix: memory leak optimize: use special ChangableChild object --- readme.md | 6 +- src/sigui/events.nim | 27 +++++- src/sigui/scrollArea.nim | 4 +- src/sigui/textArea.nim | 6 +- src/sigui/uiobj.nim | 189 ++++++++++++++++++++++++--------------- tests/t_todoapp.nim | 14 +-- 6 files changed, 156 insertions(+), 90 deletions(-) diff --git a/readme.md b/readme.md index f4d2f7f..ce40bb1 100644 --- a/readme.md +++ b/readme.md @@ -297,7 +297,7 @@ Built-in interpolation modifiers: ```nim type MyComponent = ref object of Uiobj - changableChild: CustomProperty[UiRect] + changableChild: ChangableChild[UiRect] registerComponent MyComponent @@ -324,7 +324,7 @@ Changable childs could be used to re-build component tree on any event. # ...in makeLaout macro... var elements = ["first", "second"] -var elementsObj: CustomProperty[Layout] +var elementsObj: ChangableChild[Layout] elementsObj --- Layout(): orientation = vertical @@ -355,7 +355,7 @@ elementsObj --- Layout(): The `<--- ctor: prop[]; event[]; ...` syntax can be used to re-build tree based on property changes ```nim var elements = ["first", "second"].property -var elementsObj: CustomProperty[Layout] +var elementsObj: ChangableChild[Layout] elementsObj --- Layout(): <--- Layout(): elements[] diff --git a/src/sigui/events.nim b/src/sigui/events.nim index c2a78a2..a725259 100644 --- a/src/sigui/events.nim +++ b/src/sigui/events.nim @@ -16,9 +16,12 @@ type EventBase = object connected: seq[EventConnection[int]] # type of function argument does not matter for this + FlaggedPointer* = distinct pointer + ## pointer, but first bit is used for a flag (pointers are aligned anyway) + Event*[T] = object # pointer is wrapped to an object to attach custom destructor p: ptr EventObj[T] - uiobj*: pointer # most of events is .changed properties, after most of which redraw should happen. If not nil, emit will call redrawUiobj + uiobj*: FlaggedPointer # most of events is .changed properties, after most of which redraw should happen. If not nil, emit will call redrawUiobj EventObj*[T] = object ## only EventHandler can be connected to event @@ -29,7 +32,10 @@ type connected: seq[EventConnection[T]] -var redrawUiobj*: proc(uiobj: pointer) {.cdecl.} +var redrawUiobj*: proc(uiobj: FlaggedPointer) {.cdecl.} + + +proc `==`*(a, b: FlaggedPointer): bool {.borrow.} #* ------------- Event ------------- *# @@ -38,6 +44,17 @@ proc destroyEvent(s: ptr EventBase) {.raises: [].} proc destroyEventHandler(handler: ptr EventHandlerObj) {.raises: [].} +proc `=trace`[T](event: var Event[T], env: pointer) = + if event.p != nil: + for conn in event.p[].connected.mitems: + `=trace`(conn, env) + +proc `=trace`(eh: var EventHandler, env: pointer) = + if eh.p != nil: + for event in eh.p[].connected: + `=trace`(event[], env) + + proc `=destroy`[T](s: Event[T]) = if s.p != nil: destroyEvent(cast[ptr EventBase](s.p)) @@ -64,6 +81,7 @@ proc destroyEvent(s: ptr EventBase) = handler[].connected.delete i else: inc i + `=destroy`(s[]) dealloc s @@ -86,6 +104,7 @@ proc destroyEventHandler(handler: ptr EventHandlerObj) = s[].connected.delete i else: inc i + `=destroy`(handler[]) dealloc handler @@ -154,7 +173,7 @@ proc emit*[T](s: Event[T], v: T, disableFlags: set[EventConnectionFlag] = {}) = if (disableFlags * s.p[].connected[i].flags).len == 0: s.p[].connected[i].f(v) inc i - if s.uiobj != nil: + if s.uiobj.pointer != nil: redrawUiobj s.uiobj proc emit*(s: Event[void], disableFlags: set[EventConnectionFlag] = {}) = @@ -164,7 +183,7 @@ proc emit*(s: Event[void], disableFlags: set[EventConnectionFlag] = {}) = if (disableFlags * s.p[].connected[i].flags).len == 0: s.p[].connected[i].f() inc i - if s.uiobj != nil: + if s.uiobj.pointer != nil: redrawUiobj s.uiobj diff --git a/src/sigui/scrollArea.nim b/src/sigui/scrollArea.nim index 11c0a5b..71250cb 100644 --- a/src/sigui/scrollArea.nim +++ b/src/sigui/scrollArea.nim @@ -43,7 +43,7 @@ type {ScrollAreaSetting.low..ScrollAreaSetting.high} ).property - verticalScrollbarObj*: CustomProperty[UiObj] + verticalScrollbarObj*: ChangableChild[UiObj] ## if changed, properties below can be attached by you to new object verticalScrollbarOpacity*: Property[float] ## has default animation @@ -53,7 +53,7 @@ type verticalScrollBarLastShown*: Property[Time] verticalScrollBarHideDelay*: Property[Duration] = initDuration(milliseconds = 1000).property - horizontalScrollbarObj*: CustomProperty[UiObj] + horizontalScrollbarObj*: ChangableChild[UiObj] ## if changed, properties below can be attached by you to new object horizontalScrollbarOpacity*: Property[float] ## has default animation diff --git a/src/sigui/textArea.nim b/src/sigui/textArea.nim index 6bd90cf..7dde8d0 100644 --- a/src/sigui/textArea.nim +++ b/src/sigui/textArea.nim @@ -42,9 +42,9 @@ type TextArea* = ref object of Uiobj - cursorObj*: CustomProperty[UiObj] - selectionObj*: CustomProperty[Uiobj] - textObj*: CustomProperty[UiText] + cursorObj*: ChangableChild[UiObj] + selectionObj*: ChangableChild[Uiobj] + textObj*: ChangableChild[UiText] active*: Property[bool] text*: Property[string] diff --git a/src/sigui/uiobj.nim b/src/sigui/uiobj.nim index 7640912..b7ddccc 100644 --- a/src/sigui/uiobj.nim +++ b/src/sigui/uiobj.nim @@ -17,7 +17,7 @@ type `end` Anchor* = object - obj: UiObj + obj {.cursor.}: UiObj # if nil, anchor is disabled offsetFrom: AnchorOffsetFrom offset: float32 @@ -97,6 +97,12 @@ type onTick*: Event[TickEvent] + ChangableChild*[T] = object + parent {.cursor.}: Uiobj + childIndex: int + changed*: Event[void] + + #--- Signals --- SubtreeSignal* = ref object of Signal @@ -269,8 +275,8 @@ proc redraw*(obj: Uiobj, ifVisible = true) = if win != nil: redraw win -redrawUiobj = proc(obj: pointer) {.cdecl.} = - redraw cast[Uiobj](obj) +redrawUiobj = proc(obj: FlaggedPointer) {.cdecl.} = + redraw(cast[Uiobj](cast[int](obj) and (not 1)), (cast[int](obj.pointer) and 1) == 0) proc posToLocal*(pos: Vec2, obj: Uiobj): Vec2 = @@ -483,7 +489,7 @@ proc initRedrawWhenPropertyChangedStatic[T: UiObj](this: T) = {.push, warning[Deprecated]: off.} for name, x in this[].fieldPairs: when name == "visibility": - x.changed.connectTo this: redraw(this, ifVisible=false) + x.changed.uiobj = cast[FlaggedPointer](cast[int](this) or 1) elif name == "globalX" or name == "globalY": discard # will anyway be handled in parent @@ -491,9 +497,9 @@ proc initRedrawWhenPropertyChangedStatic[T: UiObj](this: T) = elif x is Property or x is CustomProperty: when compiles(initRedrawWhenPropertyChanged_ignore(T, name)): when not initRedrawWhenPropertyChanged_ignore(T, name): - x.changed.uiobj = cast[pointer](this) + x.changed.uiobj = cast[FlaggedPointer](this) else: - x.changed.uiobj = cast[pointer](this) + x.changed.uiobj = cast[FlaggedPointer](this) {.pop.} @@ -552,8 +558,10 @@ proc initIfNeeded*(obj: Uiobj) = obj.recieve(ParentChanged(newParentInTree: obj.parent)) obj.parent.recieve(ChildAdded(child: obj)) + #--- Anchors --- + proc fillHorizontal*(this: Uiobj, obj: Uiobj, margin: float32 = 0) = this.left = obj.left + margin this.right = obj.right - margin @@ -575,19 +583,74 @@ proc fill*(this: Uiobj, obj: Uiobj, marginX: float32, marginY: float32) = this.fillVertical(obj, marginY) +#----- Layers ----- + + +proc `=destroy`(l: DrawLayer) = + if l.obj != nil: + case l.order + of before: l.obj.drawLayering.before.delete l.obj.drawLayering.before.find(UiobjCursor(obj: l.this)) + of beforeChilds: l.obj.drawLayering.beforeChilds.delete l.obj.drawLayering.beforeChilds.find(UiobjCursor(obj: l.this)) + of after: l.obj.drawLayering.after.delete l.obj.drawLayering.after.find(UiobjCursor(obj: l.this)) + +proc `=destroy`(l: DrawLayering) = + for x in l.before: + x.obj.m_drawLayer.obj = nil + x.obj.m_drawLayer = DrawLayer() + for x in l.after: + x.obj.m_drawLayer.obj = nil + x.obj.m_drawLayer = DrawLayer() + + +proc before*(this: Uiobj): Layer = + Layer(obj: this, order: LayerOrder.before) + +proc beforeChilds*(this: Uiobj): Layer = + Layer(obj: this, order: LayerOrder.beforeChilds) + +proc after*(this: Uiobj): Layer = + Layer(obj: this, order: LayerOrder.after) + + +proc `drawLayer=`*(this: Uiobj, layer: typeof nil) = + this.m_drawLayer = DrawLayer() + +proc `drawLayer=`*(this: Uiobj, layer: Layer) = + if this.m_drawLayer.obj != nil: this.drawLayer = nil + if layer.obj == nil: return + this.m_drawLayer = DrawLayer(obj: layer.obj, order: layer.order, this: this) + + case layer.order + of before: layer.obj.drawLayering.before.add UiobjCursor(obj: this) + of beforeChilds: layer.obj.drawLayering.beforeChilds.add UiobjCursor(obj: this) + of after: layer.obj.drawLayering.after.add UiobjCursor(obj: this) + + +#----- Adding childs ----- + + method deteach*(this: UiObj) {.base.} proc deteachStatic[T: UiObj](this: T) = if this == nil: return + this.drawLayer = nil + disconnect this.eventHandler - for x in this.childs: deteach(x) - {.push, warning[Deprecated]: off.} for x in this[].fields: when x is Property or x is CustomProperty: disconnect x.changed - {.pop.} + + for anchor in this.anchors.fields: + disconnect anchor.eventHandler + anchor = Anchor() + + for x in this.childs: + deteach(x) + x.parent = nil + + this.childs = @[] method deteach*(this: UiObj) {.base.} = @@ -596,9 +659,14 @@ method deteach*(this: UiObj) {.base.} = proc delete*(this: UiObj) = + if this == nil: return + deteach this + if this.parent != nil: - this.parent.childs.del this.parent.childs.find(this) + let i = this.parent.childs.find(this) + if i != -1: + this.parent.childs.delete i this.parent = nil @@ -618,7 +686,35 @@ method addChild*(parent: Uiobj, child: Uiobj) {.base.} = parent.recieve(ChildAdded(child: child)) -method addChangableChildUntyped*(parent: Uiobj, child: Uiobj): CustomProperty[Uiobj] {.base.} = +proc `val=`*[T](p: var ChangableChild[T], v: T) = + ## note: p.changed will not be emitted if new value is same as previous value + p.parent.childs[p.childIndex].parent = nil + deteach p.parent.childs[p.childIndex] + p.parent.childs[p.childIndex] = v + v.parent = p.parent + + if v.initialized: + v.recieve(ParentChanged(newParentInTree: p.parent)) + p.parent.recieve(ChildAdded(child: v)) + + emit(p.changed) + +proc `[]=`*[T](p: var ChangableChild[T], v: T) {.inline.} = p.val = v + +proc val*[T](p: ChangableChild[T]): T {.inline.} = + result = p.parent.childs[p.childIndex].T + +proc val*(p: ChangableChild[UiObj]): UiObj {.inline.} = + # we need this overload to avoid ConvFromXtoItselfNotNeeded warning + result = p.parent.childs[p.childIndex] + +proc `[]`*[T](p: ChangableChild[T]): T {.inline.} = p.val + +proc `{}`*[T](p: var ChangableChild[T]): T {.inline.} = p.val +proc `{}=`*[T](p: var ChangableChild[T], v: T) {.inline.} = p.val = v + + +method addChangableChildUntyped*(parent: Uiobj, child: Uiobj): ChangableChild[Uiobj] {.base.} = assert child != nil if parent.newChildsObject != nil: @@ -626,35 +722,24 @@ method addChangableChildUntyped*(parent: Uiobj, child: Uiobj): CustomProperty[Ui else: # add to parent.childs seq even if addChild is overrided assert child.parent == nil + child.parent = parent parent.childs.add child + if not child.attachedToWindow and parent.attachedToWindow: let win = parent.parentUiWindow if win != nil: child.recieve(AttachedToWindow(window: win)) + if child.initialized: child.recieve(ParentChanged(newParentInTree: parent)) parent.recieve(ChildAdded(child: child)) - let i = parent.childs.high - result = CustomProperty[Uiobj]( - get: proc(): Uiobj = parent.childs[i], - set: (proc(v: Uiobj) = - parent.childs[i].parent = nil - deteach parent.childs[i] - parent.childs[i] = v - v.parent = parent - if v.initialized: - v.recieve(ParentChanged(newParentInTree: parent)) - parent.recieve(ChildAdded(child: v)) - ), - ) - + result = ChangableChild[Uiobj](parent: parent, childIndex: parent.childs.high) -proc addChangableChild*[T: UiObj](parent: Uiobj, child: T): CustomProperty[T] = - var prop = parent.addChangableChildUntyped(child) - cast[ptr CustomProperty[UiObj]](result.addr)[] = move prop +proc addChangableChild*[T: UiObj](parent: Uiobj, child: T): ChangableChild[T] = + result = cast[ChangableChild[T]](parent.addChangableChildUntyped(child)) macro super*[T: Uiobj](obj: T): auto = @@ -670,49 +755,7 @@ macro super*[T: Uiobj](obj: T): auto = else: error("unexpected type impl", obj) - -#----- Layers ----- - - - -proc `=destroy`(l: DrawLayer) = - if l.obj != nil: - case l.order - of before: l.obj.drawLayering.before.delete l.obj.drawLayering.before.find(UiobjCursor(obj: l.this)) - of beforeChilds: l.obj.drawLayering.beforeChilds.delete l.obj.drawLayering.beforeChilds.find(UiobjCursor(obj: l.this)) - of after: l.obj.drawLayering.after.delete l.obj.drawLayering.after.find(UiobjCursor(obj: l.this)) - -proc `=destroy`(l: DrawLayering) = - for x in l.before: - x.obj.m_drawLayer.obj = nil - x.obj.m_drawLayer = DrawLayer() - for x in l.after: - x.obj.m_drawLayer.obj = nil - x.obj.m_drawLayer = DrawLayer() - - -proc before*(this: Uiobj): Layer = - Layer(obj: this, order: LayerOrder.before) - -proc beforeChilds*(this: Uiobj): Layer = - Layer(obj: this, order: LayerOrder.beforeChilds) - -proc after*(this: Uiobj): Layer = - Layer(obj: this, order: LayerOrder.after) - - -proc `drawLayer=`*(this: Uiobj, layer: typeof nil) = - this.m_drawLayer = DrawLayer() - -proc `drawLayer=`*(this: Uiobj, layer: Layer) = - if this.m_drawLayer.obj != nil: this.drawLayer = nil - if layer.obj == nil: return - this.m_drawLayer = DrawLayer(obj: layer.obj, order: layer.order, this: this) - - case layer.order - of before: layer.obj.drawLayering.before.add UiobjCursor(obj: this) - of beforeChilds: layer.obj.drawLayering.beforeChilds.add UiobjCursor(obj: this) - of after: layer.obj.drawLayering.after.add UiobjCursor(obj: this) +#----- Drawing ----- method draw*(win: UiWindow, ctx: DrawContext) = @@ -814,12 +857,12 @@ proc bindingImpl*( ## .. code-block:: nim ## type MyObj = ref object of Uiobj ## c: Property[int] - ## + ## ## let obj = MyObj() ## obj.binding c: ## if config.csd[]: parent[].b else: 10[] ## - ## convers to (roughly): + ## converts to (roughly): ## ## .. code-block:: nim ## block bindingBlock: diff --git a/tests/t_todoapp.nim b/tests/t_todoapp.nim index debed81..96db346 100644 --- a/tests/t_todoapp.nim +++ b/tests/t_todoapp.nim @@ -10,15 +10,18 @@ type tasks: seq[tuple[name: string, complete: Property[bool]]] tasksChanged: Event[void] - layout: CustomProperty[Layout] + layout: ChangableChild[Layout] - DestroyLogger = object + DestroyLoggerInner = object message: string + DestroyLogger = ref object of Uiobj + inner: DestroyLoggerInner + registerComponent App -proc `=destroy`(x: DestroyLogger) = +proc `=destroy`(x: DestroyLoggerInner) = if x.message != "": echo x.message @@ -103,7 +106,8 @@ test "todo app": for i in 0..app.tasks.high: template task: auto = app.tasks[i] - let logger {.used.} = DestroyLogger(message: "destroyed: " & $i) + - DestroyLogger() as logger: + this.inner.message = "destroyed: " & $i - Layout(): spacing = 10 @@ -132,6 +136,6 @@ test "todo app": this.fill parent on this.mouseDownAndUpInside: task.complete[] = not task.complete[] - echo "made sure ", logger.message, " works" + echo "made sure ", logger.inner.message, " works" run window.siwinWindow