-
Notifications
You must be signed in to change notification settings - Fork 108
Improve visualizer usability #1382
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
base: main
Are you sure you want to change the base?
Changes from all commits
e3b81f3
cbcea20
9c1fc6a
25d04a4
1ce9943
b5d3b16
6dddf94
6d2d31b
e71a870
18c3b6f
5646d10
18f82cd
07ab5fd
58c58a9
2aaa120
5ec8a18
82eae8b
1633f4c
1e1da90
8fbeab4
a50f9a5
c4fe05e
6687f04
cdfbc82
5d3e3f4
fdf2253
5280f84
a2c98ae
325b239
688688e
24fae61
cc40120
a9a44e9
4f9f92b
418abf1
f2b5169
7b7b41a
a489f00
03c505d
08f412c
543258b
4a8d29d
6a056ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,27 +1,39 @@ | ||||||
package com.squareup.workflow1.traceviewer | ||||||
|
||||||
import androidx.compose.foundation.layout.Arrangement | ||||||
import androidx.compose.foundation.layout.Box | ||||||
import androidx.compose.foundation.layout.Column | ||||||
import androidx.compose.foundation.layout.padding | ||||||
import androidx.compose.runtime.Composable | ||||||
import androidx.compose.runtime.LaunchedEffect | ||||||
import androidx.compose.runtime.getValue | ||||||
import androidx.compose.runtime.mutableFloatStateOf | ||||||
import androidx.compose.runtime.mutableIntStateOf | ||||||
import androidx.compose.runtime.mutableStateListOf | ||||||
import androidx.compose.runtime.mutableStateMapOf | ||||||
import androidx.compose.runtime.mutableStateOf | ||||||
import androidx.compose.runtime.remember | ||||||
import androidx.compose.runtime.setValue | ||||||
import androidx.compose.runtime.snapshotFlow | ||||||
import androidx.compose.runtime.snapshots.SnapshotStateMap | ||||||
import androidx.compose.ui.Alignment | ||||||
import androidx.compose.ui.Modifier | ||||||
import androidx.compose.ui.geometry.Offset | ||||||
import androidx.compose.ui.layout.onGloballyPositioned | ||||||
import androidx.compose.ui.layout.onSizeChanged | ||||||
import androidx.compose.ui.unit.IntSize | ||||||
import androidx.compose.ui.unit.dp | ||||||
import androidx.compose.ui.window.WindowPosition.PlatformDefault.x | ||||||
import com.squareup.workflow1.traceviewer.model.Node | ||||||
import com.squareup.workflow1.traceviewer.model.NodeUpdate | ||||||
import com.squareup.workflow1.traceviewer.ui.FrameSelectTab | ||||||
import com.squareup.workflow1.traceviewer.ui.RightInfoPanel | ||||||
import com.squareup.workflow1.traceviewer.ui.TraceModeToggleSwitch | ||||||
import com.squareup.workflow1.traceviewer.util.RenderTrace | ||||||
import com.squareup.workflow1.traceviewer.ui.control.DisplayDevices | ||||||
import com.squareup.workflow1.traceviewer.ui.control.FileDump | ||||||
import com.squareup.workflow1.traceviewer.ui.control.FrameNavigator | ||||||
import com.squareup.workflow1.traceviewer.ui.control.SearchBox | ||||||
import com.squareup.workflow1.traceviewer.ui.control.TraceModeToggleSwitch | ||||||
import com.squareup.workflow1.traceviewer.ui.control.UploadFile | ||||||
import com.squareup.workflow1.traceviewer.util.SandboxBackground | ||||||
import com.squareup.workflow1.traceviewer.util.UploadFile | ||||||
import com.squareup.workflow1.traceviewer.util.parser.RenderTrace | ||||||
import io.github.vinceglb.filekit.PlatformFile | ||||||
|
||||||
/** | ||||||
|
@@ -31,14 +43,20 @@ import io.github.vinceglb.filekit.PlatformFile | |||||
internal fun App( | ||||||
modifier: Modifier = Modifier | ||||||
) { | ||||||
var appWindowSize by remember { mutableStateOf(IntSize(0, 0)) } | ||||||
var selectedNode by remember { mutableStateOf<NodeUpdate?>(null) } | ||||||
val workflowFrames = remember { mutableStateListOf<Node>() } | ||||||
var frameSize by remember { mutableIntStateOf(0) } | ||||||
var rawRenderPass by remember { mutableStateOf("") } | ||||||
var frameIndex by remember { mutableIntStateOf(0) } | ||||||
val sandboxState = remember { SandboxState() } | ||||||
val nodeLocations = remember { mutableListOf<SnapshotStateMap<Node, Offset>>() } | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, the original goal was that I would only want compositions to track what's happening to each |
||||||
|
||||||
// Default to File mode, and can be toggled to be in Live mode. | ||||||
var active by remember { mutableStateOf(false) } | ||||||
var traceMode by remember { mutableStateOf<TraceMode>(TraceMode.File(null)) } | ||||||
var selectedTraceFile by remember { mutableStateOf<PlatformFile?>(null) } | ||||||
// frameIndex is set to -1 when app is in Live Mode, so we increment it by one to avoid off-by-one errors | ||||||
val frameInd = if (traceMode is TraceMode.Live) frameIndex + 1 else frameIndex | ||||||
|
||||||
LaunchedEffect(sandboxState) { | ||||||
snapshotFlow { frameIndex }.collect { | ||||||
|
@@ -47,47 +65,81 @@ internal fun App( | |||||
} | ||||||
|
||||||
Box( | ||||||
modifier = modifier | ||||||
modifier = modifier.onSizeChanged { | ||||||
appWindowSize = it | ||||||
} | ||||||
) { | ||||||
fun resetStates() { | ||||||
selectedTraceFile = null | ||||||
selectedNode = null | ||||||
frameIndex = 0 | ||||||
workflowFrames.clear() | ||||||
frameSize = 0 | ||||||
rawRenderPass = "" | ||||||
active = false | ||||||
nodeLocations.clear() | ||||||
} | ||||||
|
||||||
// Main content | ||||||
SandboxBackground( | ||||||
appWindowSize = appWindowSize, | ||||||
sandboxState = sandboxState, | ||||||
) { | ||||||
// if there is not a file selected and trace mode is live, then don't render anything. | ||||||
val readyForFileTrace = traceMode is TraceMode.File && selectedTraceFile != null | ||||||
val readyForLiveTrace = traceMode is TraceMode.Live | ||||||
val readyForFileTrace = TraceMode.validateFileMode(traceMode) | ||||||
val readyForLiveTrace = TraceMode.validateLiveMode(traceMode) | ||||||
|
||||||
if (readyForFileTrace || readyForLiveTrace) { | ||||||
active = true | ||||||
RenderTrace( | ||||||
traceSource = traceMode, | ||||||
frameInd = frameIndex, | ||||||
onFileParse = { workflowFrames.addAll(it) }, | ||||||
onNodeSelect = { node, prevNode -> | ||||||
selectedNode = NodeUpdate(node, prevNode) | ||||||
}, | ||||||
onNewFrame = { frameIndex += 1 } | ||||||
onFileParse = { frameSize += it }, | ||||||
onNodeSelect = { selectedNode = it }, | ||||||
onNewFrame = { frameIndex += 1 }, | ||||||
onNewData = { rawRenderPass += "$it," }, | ||||||
storeNodeLocation = { node, loc -> nodeLocations[frameInd] += (node to loc) } | ||||||
) | ||||||
} | ||||||
} | ||||||
|
||||||
FrameSelectTab( | ||||||
frames = workflowFrames, | ||||||
currentIndex = frameIndex, | ||||||
onIndexChange = { frameIndex = it }, | ||||||
modifier = Modifier.align(Alignment.TopCenter) | ||||||
) | ||||||
|
||||||
RightInfoPanel( | ||||||
selectedNode = selectedNode, | ||||||
Column( | ||||||
modifier = Modifier | ||||||
.align(Alignment.TopEnd) | ||||||
) | ||||||
.align(Alignment.TopCenter) | ||||||
.padding(top = 8.dp), | ||||||
verticalArrangement = Arrangement.spacedBy(8.dp), | ||||||
horizontalAlignment = Alignment.CenterHorizontally | ||||||
) { | ||||||
if (active) { | ||||||
// Since we can jump from frame to frame, we fill in the map during each recomposition | ||||||
if (nodeLocations.getOrNull(frameInd) == null) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think using square brackets to index could throw an IndexOutOfBoundsException right? Having |
||||||
// frameSize has not been updated yet, so on the first frame, frameSize = nodeLocations.size = 0, | ||||||
// and it will append a new map | ||||||
while (nodeLocations.size <= frameSize) { | ||||||
nodeLocations += mutableStateMapOf() | ||||||
} | ||||||
} | ||||||
|
||||||
val frameNodeLocations = nodeLocations[frameInd] | ||||||
SearchBox( | ||||||
nodes = frameNodeLocations.keys.toList(), | ||||||
onSearch = { name -> | ||||||
sandboxState.scale = 1f | ||||||
val node = frameNodeLocations.keys.first { it.name == name } | ||||||
val newX = (sandboxState.offset.x - frameNodeLocations.getValue(node).x | ||||||
+ appWindowSize.width / 2) | ||||||
val newY = (sandboxState.offset.y - frameNodeLocations.getValue(node).y | ||||||
+ appWindowSize.height / 2) | ||||||
sandboxState.offset = Offset(x = newX, y = newY) | ||||||
}, | ||||||
) | ||||||
|
||||||
FrameNavigator( | ||||||
totalFrames = frameSize, | ||||||
currentIndex = frameIndex, | ||||||
onIndexChange = { frameIndex = it }, | ||||||
) | ||||||
} | ||||||
} | ||||||
|
||||||
TraceModeToggleSwitch( | ||||||
onToggle = { | ||||||
|
@@ -96,13 +148,12 @@ internal fun App( | |||||
frameIndex = 0 | ||||||
TraceMode.File(null) | ||||||
} else { | ||||||
// TODO: TraceRecorder needs to be able to take in multiple clients if this is the case | ||||||
/* | ||||||
We set the frame to -1 here since we always increment it during Live mode as the list of | ||||||
frames get populated, so we avoid off by one when indexing into the frames. | ||||||
*/ | ||||||
frameIndex = -1 | ||||||
TraceMode.Live | ||||||
TraceMode.Live() | ||||||
} | ||||||
}, | ||||||
traceMode = traceMode, | ||||||
|
@@ -120,6 +171,26 @@ internal fun App( | |||||
modifier = Modifier.align(Alignment.BottomStart) | ||||||
) | ||||||
} | ||||||
|
||||||
if (traceMode is TraceMode.Live && (traceMode as TraceMode.Live).device == null) { | ||||||
DisplayDevices( | ||||||
onDeviceSelect = { selectedDevice -> | ||||||
traceMode = TraceMode.Live(selectedDevice) | ||||||
}, | ||||||
devices = listDevices(), | ||||||
modifier = Modifier.align(Alignment.Center) | ||||||
) | ||||||
|
||||||
FileDump( | ||||||
trace = rawRenderPass, | ||||||
modifier = Modifier.align(Alignment.BottomStart) | ||||||
) | ||||||
} | ||||||
|
||||||
RightInfoPanel( | ||||||
selectedNode = selectedNode, | ||||||
modifier = Modifier.align(Alignment.TopEnd) | ||||||
) | ||||||
} | ||||||
} | ||||||
|
||||||
|
@@ -134,5 +205,25 @@ internal class SandboxState { | |||||
|
||||||
internal sealed interface TraceMode { | ||||||
data class File(val file: PlatformFile?) : TraceMode | ||||||
data object Live : TraceMode | ||||||
data class Live(val device: String? = null) : TraceMode | ||||||
|
||||||
companion object { | ||||||
fun validateLiveMode(traceMode: TraceMode): Boolean { | ||||||
return traceMode is Live && traceMode.device != null | ||||||
} | ||||||
|
||||||
fun validateFileMode(traceMode: TraceMode): Boolean { | ||||||
return traceMode is File && traceMode.file != null | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
/** | ||||||
* Allows users to select from multiple devices that are currently running. | ||||||
*/ | ||||||
private fun listDevices(): List<String> { | ||||||
val process = ProcessBuilder("adb", "devices", "-l").start() | ||||||
process.waitFor() | ||||||
// We drop the header "List of devices attached" | ||||||
return process.inputStream.bufferedReader().readLines().drop(1).dropLast(1) | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's usually a bad practice to use
mutableListOf
in composition, should this bemutableStateListOf
?