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

Makes states invalidation more robust #872

Merged
merged 1 commit into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 33 additions & 21 deletions Context.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ import (
// It returns an unique value each time it is called.
func GenAutoID(id string) ID {
idx, ok := Context.widgetIndex[id]
if !ok {
Context.widgetIndex[id] = 0
return ID(id)

if ok {
idx++
}

Context.widgetIndex[id]++
Context.widgetIndex[id] = idx

return ID(fmt.Sprintf("%s##%d", id, idx))
}
Expand Down Expand Up @@ -56,6 +56,10 @@ type GIUContext struct {
// Indicate whether current application is running
isAlive bool

// when dirty is true, flushStates must be called before any GetState use
// when it is false, calling flushStates is noop
dirty bool

// States will used by custom widget to store data
state sync.Map

Expand Down Expand Up @@ -100,30 +104,32 @@ func (c *GIUContext) IO() *imgui.IO {
return imgui.CurrentIO()
}

// invalidAllState should be called before rendering.
// it ensures all states are marked as invalid for that moment.
func (c *GIUContext) invalidAllState() {
c.state.Range(func(_, v any) bool {
if s, ok := v.(*state); ok {
c.m.Lock()
s.valid = false
c.m.Unlock()
}

return true
})
// SetDirty permits MasterWindow defering setting dirty states after it's render().
func (c *GIUContext) SetDirty() {
c.dirty = true
}

// cleanState removes all states that were not marked as valid during rendering.
// should be called after rendering.
func (c *GIUContext) cleanState() {
// cleanStates removes all states that were not marked as valid during rendering,
// then reset said flag before new usage
// should always be called before first Get/Set state use in renderloop
// since afterRender() and beforeRender() are not waranted to run (see glfw_window_refresh_callback)
// we call it at the very start of our render()
// but just in case something happened, we also use the "dirty" flag to enforce (or avoid) flushing
// on critical path.
func (c *GIUContext) cleanStates() {
if !c.dirty {
return
}

c.state.Range(func(k, v any) bool {
if s, ok := v.(*state); ok {
c.m.Lock()
valid := s.valid
c.m.Unlock()

if !valid {
if valid {
s.valid = false
} else {
c.state.Delete(k)
s.data.Dispose()
}
Expand All @@ -132,8 +138,8 @@ func (c *GIUContext) cleanState() {
return true
})

// Reset widgetIndex
c.widgetIndex = make(map[string]int)
c.dirty = false
}

// Backend returns the imgui.backend used by the context.
Expand All @@ -143,16 +149,20 @@ func (c *GIUContext) Backend() backend.Backend[glfwbackend.GLFWWindowFlags] {

// SetState is a generic version of Context.SetState.
func SetState[T any, PT genericDisposable[T]](c *GIUContext, id ID, data PT) {
c.cleanStates()
c.state.Store(id, &state{valid: true, data: data})
}

// SetState stores data in context by id.
func (c *GIUContext) SetState(id ID, data Disposable) {
c.cleanStates()
c.state.Store(id, &state{valid: true, data: data})
}

// GetState is a generic version of Context.GetState.
func GetState[T any, PT genericDisposable[T]](c *GIUContext, id ID) PT {
c.cleanStates()

if s, ok := c.load(id); ok {
c.m.Lock()
s.valid = true
Expand All @@ -169,6 +179,8 @@ func GetState[T any, PT genericDisposable[T]](c *GIUContext, id ID) PT {

// GetState returns previously stored state by id.
func (c *GIUContext) GetState(id ID) any {
c.cleanStates()

if s, ok := c.load(id); ok {
c.m.Lock()
s.valid = true
Expand Down
5 changes: 3 additions & 2 deletions Context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,12 @@ func Test_invalidState(t *testing.T) {
SetState(ctx, i, s)
}

ctx.invalidAllState()
// SetState set "valid=true" so we simulate a first end of frame before tests
ctx.SetDirty()

_ = GetState[teststate](ctx, state2ID)

ctx.cleanState()
ctx.SetDirty()

assert.NotNil(t, GetState[teststate](ctx, state2ID),
"although state has been accessed during the frame, it has ben deleted by invalidAllState/cleanState")
Expand Down
5 changes: 3 additions & 2 deletions MasterWindow.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ func (w *MasterWindow) sizeChange(_, _ int) {
}

func (w *MasterWindow) beforeRender() {
Context.invalidAllState()
Context.FontAtlas.rebuildFontAtlas()

// process texture load requests
Expand All @@ -214,7 +213,6 @@ func (w *MasterWindow) beforeRender() {
}

func (w *MasterWindow) afterRender() {
Context.cleanState()
}

func (w *MasterWindow) beforeDestroy() {
Expand All @@ -223,6 +221,9 @@ func (w *MasterWindow) beforeDestroy() {
}

func (w *MasterWindow) render() {
Context.cleanStates()
defer Context.SetDirty()

fin := w.setTheme()
defer fin()

Expand Down
Loading