go-p5 / p5

p5 is a simple package that provides primitives resembling the ones exposed by p5js.org
https://go-p5.github.io/
BSD 3-Clause "New" or "Revised" License
146 stars 12 forks source link

p5: send custom events in order to control the behavior of the draw loop #35

Closed psampaz closed 3 years ago

psampaz commented 3 years ago

closes #34

codecov[bot] commented 3 years ago

Codecov Report

Merging #35 (4ee1410) into main (fd4fc67) will increase coverage by 6.18%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #35      +/-   ##
==========================================
+ Coverage   75.61%   81.80%   +6.18%     
==========================================
  Files           9        9              
  Lines         570      577       +7     
==========================================
+ Hits          431      472      +41     
+ Misses        119       86      -33     
+ Partials       20       19       -1     
Impacted Files Coverage Δ
proc.go 72.16% <100.00%> (+19.22%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update fd4fc67...9b4dda8. Read the comment docs.

sbinet commented 3 years ago

well, "of course" the events interface needs to be somewhat expanded to, say:

type gioWindow interface {
    Events() <-chan event.Event
    Invalidate()
    Close()
}
psampaz commented 3 years ago

using the interface is like a much cleaner way to separate tests from normal code. I will give it a try.

psampaz commented 3 years ago

@sbinet please have a look

https://github.com/go-p5/p5/pull/35/commits/8e0e9d3251929209a0ef310f1e82828cb3ca9ae4 : Solution using Proc event field https://github.com/go-p5/p5/pull/35/commits/9b4dda8223424482b0810eeb70ac38a63087602d : Solution using the interface

the data race in the failing test is related with the demo test here https://github.com/go-p5/p5/pull/35/files#diff-9a6ab27c824fb3a6082d5d518db17f3c310f1eff5def98ca5f5158309e92a08bR334. I didn't have the time to look into it yet.

sbinet commented 3 years ago

if you prefer, I can send a PR with "my" alternate design (that was heavily inspired from your interesting out-of-the-box thinking) and we can compare notes.

as you prefer.

psampaz commented 3 years ago

Yes please do so, so we can avoid the back and forth discussions.

I didn't actually spend too much time on this, I just wanted to see if my understanding was close to what you suggested.

btw I used the extra window field in order to be able to access the out event channel directly, since the Event() method of the interface has to return a receive only channel (https://github.com/gioui/gio/blob/main/app/window.go#L112)

psampaz commented 3 years ago

closing this in favour of #37