Skip to content

Commit 2c192bb

Browse files
pusolitopusolito
committed
- [Fix][Desktop] Removed skiko.vsync.enabled flag
- [Fix][Desktop] Bugs in View render ordering - [Test][Desktop] New unit tests for `RealGraphicsSurface`
1 parent 93e70e9 commit 2c192bb

File tree

9 files changed

+323
-95
lines changed

9 files changed

+323
-95
lines changed

‎Core/src/jvmTest/kotlin/io/nacular/doodle/drawing/impl/RenderManagerImplTests.kt‎

Lines changed: 70 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ import io.nacular.doodle.core.Display
1717
import io.nacular.doodle.core.InternalDisplay
1818
import io.nacular.doodle.core.Layout
1919
import io.nacular.doodle.core.View
20+
import io.nacular.doodle.core.forceBounds
2021
import io.nacular.doodle.core.forceSize
22+
import io.nacular.doodle.core.forceWidth
2123
import io.nacular.doodle.core.forceX
2224
import io.nacular.doodle.core.view
2325
import io.nacular.doodle.drawing.AffineTransform.Companion.Identity
@@ -599,7 +601,7 @@ class RenderManagerImplTests {
599601
expect(true) { grandChild in parent.children }
600602
}
601603

602-
@Test fun `rerenders on bounds changed`() {
604+
@Test fun `re-renders on bounds changed`() {
603605
val view = spyk<View> { suggestSize(Size(100)) }
604606
val display = display(view)
605607
val renderManager = renderManager(display)
@@ -611,7 +613,7 @@ class RenderManagerImplTests {
611613
verify(exactly = 2) { view.render(any()) }
612614
}
613615

614-
@Test fun `rerenders on becoming visible`() {
616+
@Test fun `re-renders on becoming visible`() {
615617
val view = spyk<View> { suggestSize(Size(100)) }
616618
val display = display(view)
617619

@@ -626,7 +628,7 @@ class RenderManagerImplTests {
626628
verifyChildAddedProperly(renderManager, display, view)
627629
}
628630

629-
@Test fun `rerenders on added becoming visible`() {
631+
@Test fun `re-renders on added becoming visible`() {
630632
val parent = container()
631633
val view = spyk<View> { suggestSize(Size(100)) }
632634
val display = display(parent)
@@ -1260,36 +1262,36 @@ class RenderManagerImplTests {
12601262
}
12611263
}
12621264

1263-
// @Test fun `cold display rect call works`() {
1264-
// data class Data(
1265-
// val grandParent: Rectangle,
1266-
// val parent : Rectangle,
1267-
// val child : Rectangle,
1268-
// val event : Pair<Rectangle, Rectangle>,
1269-
// val operation : (View, View, View) -> Unit
1270-
// )
1271-
//
1272-
// listOf(
1273-
// Data(Rectangle(100, 100), Rectangle(50, 50), Rectangle(10, 10), Rectangle(10, 10) to Rectangle(10, 0, 0, 10)) { _,parent,_ ->
1274-
// parent.x = -10.0
1275-
// },
1276-
// Data(Rectangle(100, 100), Rectangle( 0, 50), Rectangle(10, 10), Rectangle( 0, 10) to Rectangle( 10, 10)) { _,parent,_ ->
1277-
// parent.width = 1000.0
1278-
// }
1279-
// ).forEach {
1280-
// val child = view()
1281-
// val parent = container().apply { children += child; bounds = it.parent }
1282-
// val grandParent = container().apply { children += parent; bounds = it.grandParent }
1283-
// val display = display(grandParent)
1284-
// val renderManager = renderManager(display)
1285-
//
1286-
// it.operation(grandParent, parent, child)
1287-
//
1288-
// expect(it.event.second) {
1289-
// renderManager.first.displayRect(child)
1290-
// }
1291-
// }
1292-
// }
1265+
@Test fun `cold display rect call works`() {
1266+
data class Data(
1267+
val grandParent: Rectangle,
1268+
val parent : Rectangle,
1269+
val child : Rectangle,
1270+
val event : Pair<Rectangle, Rectangle>,
1271+
val operation : (View, View, View) -> Unit
1272+
)
1273+
1274+
listOf(
1275+
Data(Rectangle(100, 100), Rectangle(50, 50), Rectangle(10, 10), Rectangle(10, 10) to Rectangle(10, 0, 0, 10)) { _,parent,_ ->
1276+
parent.forceX(-10.0)
1277+
},
1278+
Data(Rectangle(100, 100), Rectangle( 0, 50), Rectangle(10, 10), Rectangle( 0, 10) to Rectangle( 10, 10)) { _,parent,_ ->
1279+
parent.forceWidth(1000.0)
1280+
}
1281+
).forEach {
1282+
val child = view()
1283+
val parent = container().apply { children += child; forceBounds(it.parent ) }
1284+
val grandParent = container().apply { children += parent; forceBounds(it.grandParent) }
1285+
val display = display(grandParent)
1286+
val renderManager = renderManager(display)
1287+
1288+
it.operation(grandParent, parent, child)
1289+
1290+
expect(it.event.second) {
1291+
renderManager.first.displayRect(child)
1292+
}
1293+
}
1294+
}
12931295

12941296
@Test fun `stops monitoring display rect when disabled`() {
12951297
val child = spyk(view()).apply {
@@ -1314,6 +1316,41 @@ class RenderManagerImplTests {
13141316
}
13151317
}
13161318

1319+
@Test fun `graphics index correct child becomes visible`() {
1320+
val child1 = spyk(view())
1321+
val child2 = spyk(view().apply { visible = false })
1322+
val child3 = spyk(view())
1323+
val parent = spyk<Container> { suggestSize(Size(10)); children += listOf(child1, child2, child3) }
1324+
val display = display(parent)
1325+
1326+
var indexes = mutableMapOf<GraphicsSurface, Int>()
1327+
val capturedIndex = slot<Int>()
1328+
1329+
val surface1 = mockk<GraphicsSurface> { every { index = capture(capturedIndex) } answers { indexes[this@mockk] = capturedIndex.captured } }
1330+
val surface3 = mockk<GraphicsSurface> { every { index = capture(capturedIndex) } answers { indexes[this@mockk] = capturedIndex.captured } }
1331+
val surface2 = mockk<GraphicsSurface> { every { index = capture(capturedIndex) } answers { indexes[this@mockk] = capturedIndex.captured } }
1332+
1333+
val scheduler = ManualAnimationScheduler()
1334+
1335+
val renderManager = renderManager(
1336+
display = display,
1337+
scheduler = scheduler,
1338+
graphicsDevice = graphicsDevice(mapOf(child1 to surface1, child2 to surface2, child3 to surface3))
1339+
)
1340+
1341+
scheduler.runJobs()
1342+
1343+
verifyChildAddedProperly(renderManager, display, parent)
1344+
1345+
expect(listOf(0,null,2)) { listOf(indexes[surface1], indexes[surface2], indexes[surface3]) }
1346+
1347+
child2.visible = true
1348+
1349+
scheduler.runJobs()
1350+
1351+
expect(listOf(0,1,2)) { listOf(indexes[surface1], indexes[surface2], indexes[surface3]) }
1352+
}
1353+
13171354
private fun verifyLayout(layout: Layout = layout(), count: Int = 2, block: (View) -> Unit) {
13181355
val container = spyk<Container>("xyz").apply { suggestSize(Size(100)) }
13191356
val child = view()

‎Desktop/src/jvmMain/kotlin/io/nacular/doodle/application/Application.kt‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ private open class ApplicationHolderImpl protected constructor(
9292
System.setProperty("skiko.rendering.laf.global", "true" )
9393
System.setProperty("skiko.rendering.useScreenMenuBar", "true" )
9494
System.setProperty("skiko.linux.autodpi", "true" )
95-
System.setProperty("skiko.vsync.enabled", "false")
95+
// System.setProperty("skiko.vsync.enabled", "false")
9696
// System.setProperty("skiko.fps.enabled", "true" )
9797

9898
UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName())
@@ -136,8 +136,12 @@ private open class ApplicationHolderImpl protected constructor(
136136
defaultFont = instance(),
137137
uiDispatcher = instance(),
138138
fontCollection = instance(),
139-
graphicsDevices = { onPaintNeeded ->
140-
RealGraphicsDevice(RealGraphicsSurfaceFactory(instance(), instance(), onPaintNeeded))
139+
graphicsDevices = { parent ->
140+
RealGraphicsDevice(RealGraphicsSurfaceFactory(
141+
parent = parent,
142+
defaultFont = instance(),
143+
fontCollection = instance(),
144+
))
141145
},
142146
renderManagers = { display ->
143147
DesktopRenderManagerImpl(display, instance(), instanceOrNull(), instanceOrNull())

‎Desktop/src/jvmMain/kotlin/io/nacular/doodle/core/WindowImpl.kt‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import io.nacular.doodle.core.impl.DisplayImpl
1111
import io.nacular.doodle.core.impl.DisplaySkiko
1212
import io.nacular.doodle.drawing.GraphicsDevice
1313
import io.nacular.doodle.drawing.RenderManager
14+
import io.nacular.doodle.drawing.impl.GraphicsSurfaceParent
1415
import io.nacular.doodle.drawing.impl.RealGraphicsDevice
1516
import io.nacular.doodle.drawing.impl.RealGraphicsSurface
1617
import io.nacular.doodle.drawing.impl.SwingCanvas
@@ -51,7 +52,7 @@ internal class WindowImpl(
5152
uiDispatcher : CoroutineDispatcher,
5253
fontCollection : FontCollection,
5354
accessibilityManager: () -> AccessibilityManagerSkiko?,
54-
graphicsDevices : (() -> Unit) -> RealGraphicsDevice<RealGraphicsSurface>,
55+
graphicsDevices : (GraphicsSurfaceParent) -> RealGraphicsDevice<RealGraphicsSurface>,
5556
private val renderManagers : (DisplayImpl ) -> RenderManager,
5657
displays : DisplayFactory,
5758
size : Size,
@@ -172,9 +173,19 @@ internal class WindowImpl(
172173

173174
val skiaLayer: JPanel get() = display.panel
174175

175-
val graphicsDevice: GraphicsDevice<RealGraphicsSurface> = graphicsDevices {
176-
display.paintNeeded()
177-
}
176+
val graphicsDevice: GraphicsDevice<RealGraphicsSurface> = graphicsDevices(
177+
object: GraphicsSurfaceParent {
178+
override val released = false
179+
override val children get() = display.surfaces
180+
181+
override fun add (child: RealGraphicsSurface) { children += child }
182+
override fun remove(child: RealGraphicsSurface) { children -= child }
183+
184+
override fun needsRerender() {
185+
display.paintNeeded()
186+
}
187+
}
188+
)
178189

179190
override val closed: ChangeObservers<WindowImpl> = ChangeObserversImpl(this)
180191

‎Desktop/src/jvmMain/kotlin/io/nacular/doodle/core/impl/DisplayImpl.kt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,8 @@ internal class DisplayImpl(
112112
}
113113
} }
114114

115+
override val surfaces = mutableListOf<RealGraphicsSurface>()
116+
115117
private inner class ChildObserversImpl: SetPool<ChildObserver<Display>>() {
116118
operator fun invoke(differences: Differences<View>) = forEach { it(this@DisplayImpl, differences) }
117119
}
@@ -208,8 +210,8 @@ internal class DisplayImpl(
208210
}
209211
}
210212

211-
children.forEach {
212-
device[it].onRender(canvas)
213+
surfaces.forEach {
214+
it.onRender(canvas)
213215
}
214216

215217
popUps.forEach {

‎Desktop/src/jvmMain/kotlin/io/nacular/doodle/core/impl/DisplaySkiko.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ internal interface DisplaySkiko: InternalDisplay {
2323
val locationOnScreen: Point
2424
val indexInParent : Int
2525
val panel : JPanel
26+
val surfaces : MutableList<RealGraphicsSurface>
2627

2728
fun syncSize ()
2829
fun shutdown ()

‎Desktop/src/jvmMain/kotlin/io/nacular/doodle/drawing/impl/RealGraphicsSurface.kt‎

Lines changed: 39 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import io.nacular.doodle.skia.skia
1313
import io.nacular.doodle.skia.skia33
1414
import io.nacular.doodle.utils.addOrAppend
1515
import io.nacular.doodle.utils.observable
16+
import org.jetbrains.annotations.VisibleForTesting
1617
import org.jetbrains.skia.ClipMode.INTERSECT
1718
import org.jetbrains.skia.Font
1819
import org.jetbrains.skia.Matrix44
@@ -23,39 +24,46 @@ import org.jetbrains.skia.paragraph.FontCollection
2324
import kotlin.properties.ReadWriteProperty
2425
import org.jetbrains.skia.Canvas as SkiaCanvas
2526

27+
internal interface GraphicsSurfaceParent {
28+
val released: Boolean
29+
val children: MutableList<RealGraphicsSurface>
30+
31+
fun add (child: RealGraphicsSurface)
32+
fun remove (child: RealGraphicsSurface)
33+
fun needsRerender( )
34+
}
35+
2636
/**
2737
* Created by Nicholas Eddy on 5/19/21.
2838
*/
2939
internal class RealGraphicsSurface(
40+
private val parent : GraphicsSurfaceParent,
3041
private val defaultFont : Font,
3142
private val fontCollection: FontCollection,
32-
private val parent : RealGraphicsSurface?,
33-
private val onPaintNeeded : () -> Unit,
34-
): GraphicsSurface {
43+
): GraphicsSurface, GraphicsSurfaceParent {
3544

36-
private var picture = null as Picture?
37-
private val children = mutableListOf<RealGraphicsSurface>()
38-
private var renderBlock by redrawProperty<((Canvas) -> Unit)?>(null)
39-
private var finalTransform: AffineTransform by parentRedrawProperty(Identity)
40-
private val pictureRecorder = PictureRecorder()
45+
private var picture = null as Picture?
46+
override val children = mutableListOf<RealGraphicsSurface>()
47+
private var renderBlock by redrawProperty<((Canvas) -> Unit)?>(null)
48+
private var finalTransform: AffineTransform by parentRedrawProperty(Identity)
49+
private val pictureRecorder = PictureRecorder()
4150

4251
internal var postRender: ((Canvas) -> Unit)? = null
4352

4453
init {
45-
parent?.add(this)
54+
parent.add(this)
4655
}
4756

4857
override var size by redrawProperty(Empty)
49-
override var index = 0
50-
set(new) {
51-
parent?.let {
52-
it.children.remove(this)
53-
if (new >= 0) {
54-
it.children.addOrAppend(new, this)
55-
}
56-
updateParentChildrenSort()
57-
}
58+
override var index = 0; set(new) {
59+
field = new
60+
61+
parent.children.remove(this)
62+
if (new >= 0) {
63+
parent.children.addOrAppend(new, this)
5864
}
65+
updateParentChildrenSort()
66+
}
5967
override var zOrder by parentRedrawProperty (0 ) { _,_ -> updateParentChildrenSort( ) }
6068
override var visible by redrawProperty (true )
6169
override var opacity by redrawProperty (0.5f )
@@ -70,7 +78,7 @@ internal class RealGraphicsSurface(
7078
renderBlock = block
7179
}
7280

73-
private var released = false
81+
override var released = false
7482

7583
override fun release() {
7684
if (!released) {
@@ -84,11 +92,9 @@ internal class RealGraphicsSurface(
8492
it.release()
8593
}
8694

87-
when {
88-
parent == null -> onPaintNeeded()
89-
parent.released == false -> { // FIXME: Should always be true
90-
parent.remove(this)
91-
}
95+
// FIXME: Should always be true
96+
if (parent.released == false) { // FIXME: Should always be true
97+
parent.remove(this)
9298
}
9399
}
94100
}
@@ -111,34 +117,30 @@ internal class RealGraphicsSurface(
111117
}
112118

113119
private fun updateParentChildrenSort() {
114-
parent?.let {
115-
it.children.sortWith { a, b -> (a.zOrder - b.zOrder).takeUnless { it == 0 } ?: (a.index - b.index) }
116-
// it.children.sortWith(Comparator<RealGraphicsSurface> { a, b -> a.zOrder - b.zOrder }.thenComparing { a, b -> a.index - b.index })
117-
it.needsRerender()
118-
}
120+
parent.children.sortWith { a, b -> (a.zOrder - b.zOrder).takeUnless { it == 0 } ?: (a.index - b.index) }
121+
parent.needsRerender()
119122
}
120123

121-
private fun add(child: RealGraphicsSurface) {
124+
@VisibleForTesting
125+
override fun add(child: RealGraphicsSurface) {
122126
children += child
123127
needsRerender()
124128
}
125129

126-
private fun remove(child: RealGraphicsSurface) {
130+
@VisibleForTesting
131+
override fun remove(child: RealGraphicsSurface) {
127132
children -= child
128133
needsRerender()
129134
}
130135

131-
private fun needsRerender() {
136+
override fun needsRerender() {
132137
try {
133138
picture?.close()
134139
} catch (_: Throwable) {}
135140

136141
picture = null
137142

138-
when (parent) {
139-
null -> onPaintNeeded()
140-
else -> parent.needsRerender()
141-
}
143+
parent.needsRerender()
142144
}
143145

144146
private fun drawToCanvas(skiaCanvas: SkiaCanvas) {
@@ -216,9 +218,6 @@ internal class RealGraphicsSurface(
216218
private fun <T> parentRedrawProperty(initial: T, onChange: RealGraphicsSurface.(old: T, new: T) -> Unit = { _,_ -> }): ReadWriteProperty<RealGraphicsSurface, T> = observable(initial) { old, new ->
217219
onChange(old, new)
218220

219-
when (parent) {
220-
null -> onPaintNeeded()
221-
else -> parent.needsRerender()
222-
}
221+
parent.needsRerender()
223222
}
224223
}

0 commit comments

Comments
 (0)