Reusing canvas for rendering - #394
Conversation
538843c to
7965f79
Compare
| |One | ||
| |Two | ||
| |One $s | ||
| |Two $s |
There was a problem hiding this comment.
This seems worse. I don't think we need to optimize statics to reuse the same surface since they're rare, and even when used there's probably only one.
There was a problem hiding this comment.
Yes, I know it looks worse here. I did this consciously, because I made #439 where I delete empty characters at the end of lines, which, in my opinion, eliminates this problem later.
There was a problem hiding this comment.
I think trimming at the end makes sense for an individual surface, especially the main one. I'll get to that PR soon.
But by combining the static surfaces we're now doing more work to merge them and allocate more space, only to then have to process more to trim the new trailing spaces. And ultimately it results in no change in the output but more code to maintain.
Since in practice there aren't multiple static nodes, and even if there are they're all not rendering at the same time, and even if multiple render they're not rendering much, doing this still doesn't gain us anything.
| private var fullWidth: Int = width | ||
| private var fullHeight: Int = height | ||
|
|
||
| private val rows = ArrayList(fullHeight) { ArrayList(fullWidth) { TextPixel(' ') } } |
There was a problem hiding this comment.
I don't see a big motivation from switching from Array here. Is it because of row resizing? We can accomplish that with a helper https://pl.kotl.in/c_ED2lc0h. I'll file a Kotlin issue to provide such an overload.
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes, I changed it only to change the size out of the box.
7965f79 to
c801320
Compare
EpicDima
changed the title
c801320 to
629ea04
Compare
|
Are you okay if I close this? Or do you want to push back more on my claim that this is an uncommon problem? |
|
No problem if you don't think it's necessary 🙂 |
2 participants
Partial solution for #153.