From cee045bf92fb185011265f138556b3b17e4e4a0e Mon Sep 17 00:00:00 2001 From: Viktor Date: Sat, 20 Jun 2020 23:30:01 +0200 Subject: [PATCH] gpu: build gpu data also when outside window This commit fixes a bug where a shape first drawn off-screen and later moved into screen would not display properly. Since we cache CPU operations (vertex transform / construction) we need to upload the constructed data to the GPU after it was build, or a later frame will use non-initialized memory for it's draw call. Note that this fix removes the optimization of not processing clip paths outside the screen - but this is assumed to be uncommon except when it is first drawn off screen to later be moved in (e.g. in a scrolling list) in which case we do want to upload the data and prepare for that later call. This commit also does a few minor clean ups and adds a test case. Signed-off-by: Viktor --- gpu/caches.go | 5 +- gpu/gpu.go | 7 +- internal/ops/reader.go | 4 +- .../rendertest/refs/TestBuildOffscreen.png | Bin 0 -> 364 bytes .../rendertest/refs/TestBuildOffscreen_1.png | Bin 0 -> 929 bytes internal/rendertest/render_test.go | 46 +++++++++++++- internal/rendertest/util_test.go | 60 +++++++++++++++++- 7 files changed, 108 insertions(+), 14 deletions(-) create mode 100644 internal/rendertest/refs/TestBuildOffscreen.png create mode 100644 internal/rendertest/refs/TestBuildOffscreen_1.png diff --git a/gpu/caches.go b/gpu/caches.go index 7a383c20..cefba837 100644 --- a/gpu/caches.go +++ b/gpu/caches.go @@ -28,8 +28,9 @@ type opCache struct { type opCacheValue struct { data pathData bounds f32.Rectangle - key ops.Key - keep bool + // the fields below are handled by opCache + key ops.Key + keep bool } func newResourceCache() *resourceCache { diff --git a/gpu/gpu.go b/gpu/gpu.go index 0f5874d9..48a6334d 100644 --- a/gpu/gpu.go +++ b/gpu/gpu.go @@ -706,9 +706,9 @@ func (d *drawOps) addClipPath(state *drawState, aux []byte, auxKey ops.Key, boun // split a transform into two parts, one which is pur offset and the // other representing the scaling, shearing and rotation part func splitTransform(t f32.Affine2D) (srs f32.Affine2D, offset f32.Point) { - sx, hx, ox, sy, hy, oy := t.Elems() + sx, hx, ox, hy, sy, oy := t.Elems() offset = f32.Point{X: ox, Y: oy} - srs = f32.NewAffine2D(sx, hx, 0, sy, hy, 0) + srs = f32.NewAffine2D(sx, hx, 0, hy, sy, 0) return } @@ -752,9 +752,6 @@ loop: auxKey.SetTransform(trans) } state.clip = state.clip.Intersect(op.bounds.Add(off)) - if state.clip.Empty() { - continue - } d.addClipPath(&state, aux, auxKey, op.bounds, off) aux = nil auxKey = ops.Key{} diff --git a/internal/ops/reader.go b/internal/ops/reader.go index cf768c01..5d713db3 100644 --- a/internal/ops/reader.go +++ b/internal/ops/reader.go @@ -62,11 +62,11 @@ func (r *Reader) Reset(ops *op.Ops) { } func (k Key) SetTransform(t f32.Affine2D) Key { - sx, hx, sy, hy, _, _ := t.Elems() + sx, hx, _, hy, sy, _ := t.Elems() k.sx = sx k.hx = hx - k.sy = sy k.hy = hy + k.sy = sy return k } diff --git a/internal/rendertest/refs/TestBuildOffscreen.png b/internal/rendertest/refs/TestBuildOffscreen.png new file mode 100644 index 0000000000000000000000000000000000000000..9ed91358c5102f5423f4aee5bc3e241445ea011c GIT binary patch literal 364 zcmeAS@N?(olHy`uVBq!ia0vp^4Is?H1SEZ8zRh7^U^Mo0aSW-L^X6h8=K%wTqX!IM z-T$<@=XZvmqeA*^@#ohW*Jm?aV_3r&!MuSjf%kw^gDJxZ7RUnz-2eZJSXdY~s>`MV OISihzelF{r5}E+9Ep1l- literal 0 HcmV?d00001 diff --git a/internal/rendertest/refs/TestBuildOffscreen_1.png b/internal/rendertest/refs/TestBuildOffscreen_1.png new file mode 100644 index 0000000000000000000000000000000000000000..881341152ee24a98dacf18b20250b4c27284210e GIT binary patch literal 929 zcmeAS@N?(olHy`uVBq!ia0vp^4Is?H1SEZ8zRh7^V9xM#aSW-L^X9gF-lHfP_rk~D zT~0H3bSb#{cd_U&2P*FpS<@8SaNQw&_AJ>Aj6RDz`~y84#T-=bNO;Ssy8JYs*`oMk z-s_p~m(T3GA2Z)js%HKFvzyM`bexu2US59v`t|?zyLRoGb^dwZV-u;~&6_v>mzz6p zo}8@gRIjD6b^-HJ;v?6FO+LBg_S<)#GVkV<8_ir3wp!mlY?~do!0|$fwyjZb%NJkE z+G@b_*urebghjtn8#KCZz5Nzvesyx$ZduofH=l}}XJ@=*^0oHA_4I$H_rG5@QJogX z^PXA7(@2Jozx3<&w!?{&R4y>(ephptwQSyV*>A7%E9Wi${lGb&iN$fk@vHZ_ni`^) z?e#jppZVG6%FQ>==r=sJ2oy1u>Wy(1>D&ERx$kk^{`ucG)a{o)UU)B#dh1K7DUWtgdEVed02E=hLE{*I&<+ zx%v9*si&JxcA8yo<9YM2++k^uBoABKGhR04*uUo6tS7(zdh5Xdv(am>Rqb6?p7{1t zkzR(`?5DB`X0sP|Z@qYWaYx$b$^*&^?LXJrzb|%g{j#gAPi@xse+BQ-Hd|IOpZ{z& zn|F)D?b;ffzYXm!%?cOx|Mh>t%6mh6y+-fx$A9AjrTULoxn_!2dGd!{%3if5mf6E^ zi#ks*!wjYaA`HtI3|Jd98GJx~9I`@h+=036|Nnh^Zb$#U`7#5@VeoYIb6Mw<&;$Tx Cj*xEv literal 0 HcmV?d00001 diff --git a/internal/rendertest/render_test.go b/internal/rendertest/render_test.go index fe0e78be..b14d089e 100644 --- a/internal/rendertest/render_test.go +++ b/internal/rendertest/render_test.go @@ -15,7 +15,7 @@ func TestTransformMacro(t *testing.T) { // testcase resulting from original bug when rendering layout.Stacked // pre-build the text - c := createText() + c := constSqPath() run(t, func(o *op.Ops) { @@ -101,7 +101,7 @@ func TestNoClipFromPaint(t *testing.T) { }) } -func createText() op.CallOp { +func constSqPath() op.CallOp { innerOps := new(op.Ops) m := op.Record(innerOps) builder := clip.Path{} @@ -115,6 +115,14 @@ func createText() op.CallOp { return m.Stop() } +func constSqCirc() op.CallOp { + innerOps := new(op.Ops) + m := op.Record(innerOps) + clip.Rect{Rect: f32.Rect(0, 0, 40, 40), + NW: 20, NE: 20, SW: 20, SE: 20}.Add(innerOps) + return m.Stop() +} + func drawChild(ops *op.Ops, text op.CallOp) op.CallOp { r1 := op.Record(ops) text.Add(ops) @@ -123,7 +131,7 @@ func drawChild(ops *op.Ops, text op.CallOp) op.CallOp { } func TestReuseStencil(t *testing.T) { - txt := createText() + txt := constSqPath() run(t, func(ops *op.Ops) { c1 := drawChild(ops, txt) c2 := drawChild(ops, txt) @@ -142,3 +150,35 @@ func TestReuseStencil(t *testing.T) { r.expect(5, 55, colornames.Black) }) } + +func TestBuildOffscreen(t *testing.T) { + // Check that something we in one frame build outside the screen + // still is rendered correctly if moved into the screen in a later + // frame. + + txt := constSqCirc() + draw := func(off float32, o *op.Ops) { + s := op.Push(o) + op.TransformOp{}.Offset(f32.Pt(0, off)).Add(o) + txt.Add(o) + paint.PaintOp{Rect: f32.Rect(0, 0, 40, 40)}.Add(o) + s.Pop() + } + + multiRun(t, + frame( + func(ops *op.Ops) { + draw(-100, ops) + }, func(r result) { + r.expect(5, 5, colornames.White) + r.expect(20, 20, colornames.White) + }), + frame( + func(ops *op.Ops) { + draw(0, ops) + }, func(r result) { + r.expect(2, 2, colornames.White) + r.expect(20, 20, colornames.Black) + r.expect(38, 38, colornames.White) + })) +} diff --git a/internal/rendertest/util_test.go b/internal/rendertest/util_test.go index a8754be4..46d10177 100644 --- a/internal/rendertest/util_test.go +++ b/internal/rendertest/util_test.go @@ -9,6 +9,7 @@ import ( "image/png" "io/ioutil" "path/filepath" + "strconv" "testing" "gioui.org/app/headless" @@ -61,9 +62,10 @@ func run(t *testing.T, f func(o *op.Ops), c func(r result)) { img, err = drawImage(128, ops, f) if err != nil { t.Error("error rendering:", err) + return } // check for a reference image and make sure we are identical. - ok = ok && verifyRef(t, img) + ok = ok && verifyRef(t, img, 0) c(result{t: t, img: img}) } @@ -74,9 +76,62 @@ func run(t *testing.T, f func(o *op.Ops), c func(r result)) { } } -func verifyRef(t *testing.T, img *image.RGBA) (ok bool) { +func frame(f func(o *op.Ops), c func(r result)) frameT { + return frameT{f: f, c: c} +} + +type frameT struct { + f func(o *op.Ops) + c func(r result) +} + +// multiRun is used to run test cases over multiple frames, typically +// to test caching interactions. +func multiRun(t *testing.T, frames ...frameT) { + // draw a few times and check that it is correct each time, to + // ensure any caching effects still generate the correct images. + var img *image.RGBA + var err error + sz := image.Point{X: 128, Y: 128} + w, err := headless.NewWindow(sz.X, sz.Y) + if err != nil { + t.Error("error creating window:", err) + t.FailNow() + } + ops := new(op.Ops) + for i := range frames { + ops.Reset() + frames[i].f(ops) + w.Frame(ops) + img, err = w.Screenshot() + if err != nil { + t.Error("error rendering:", err) + return + } + // check for a reference image and make sure we are identical. + ok := verifyRef(t, img, i) + if frames[i].c != nil { + frames[i].c(result{t: t, img: img}) + } + if *dumpImages || !ok { + name := t.Name() + ".png" + if i != 0 { + name = t.Name() + "_" + strconv.Itoa(i) + ".png" + } + if err := saveImage(name, img); err != nil { + t.Error(err) + } + } + } + +} + +func verifyRef(t *testing.T, img *image.RGBA, frame int) (ok bool) { // ensure identical to ref data path := filepath.Join("refs", t.Name()+".png") + if frame != 0 { + path = filepath.Join("refs", t.Name()+"_"+strconv.Itoa(frame)+".png") + } b, err := ioutil.ReadFile(path) if err != nil { t.Error("could not open ref:", err) @@ -102,6 +157,7 @@ func verifyRef(t *testing.T, img *image.RGBA) (ok bool) { c1, c2 := ref.RGBAAt(x, y), img.RGBAAt(x, y) if !colorsClose(c1, c2) { t.Error("not equal to ref at", x, y, " ", c1, c2) + return false } } }