canvas read/write + multiplayer overwrite bug fix #10899
Conversation
| config(byte[].class, (CanvasBuild build, byte[] bytes) -> { | ||
| if(build.data.length == bytes.length){ | ||
| build.data = bytes; | ||
| for(int i = 0; i < bytes.length; ++i){ |
There was a problem hiding this comment.
Could use System.arraycopy
| public void setPixel(int pos, int index){ | ||
| if(pos < canvasSize * canvasSize && pos >= 0 && index >= 0 && index < palette.length){ | ||
| setByte(data, pos * bitsPerPixel, index); | ||
| updateTexture(); |
There was a problem hiding this comment.
Redrawing the entire texture each time every update would result in unnecessary performance loss especially if the canvas is off screen, could use a flag to mark it was updated and have the texture updated when it's actually drawn
There was a problem hiding this comment.
had the same idea though I didn't implement it
| Dialog dialog = new Dialog(); | ||
|
|
||
| Pixmap pix = makePixmap(data); | ||
| Pixmap pix = makePixmap(data, new Pixmap(canvasSize, canvasSize)); |
There was a problem hiding this comment.
The Pixmap is never freed.
|
|
||
| Pixmap pix = makePixmap(data); | ||
| Pixmap pix = makePixmap(data, new Pixmap(canvasSize, canvasSize)); | ||
| Texture texture = new Texture(pix); |
There was a problem hiding this comment.
Texture isn't disposed when cancelling the edit.
There was a problem hiding this comment.
I don't think I touched the texture so that's not part of my PR
|
|
||
| dialog.resized(dialog::hide); | ||
| dialog.resized(() -> { | ||
| texture.dispose(); |
There was a problem hiding this comment.
Instead of adding texture/pixmap disposing in 4 places, you should add a hide listener and do it there (dialog.hidden(() => {}))
There was a problem hiding this comment.
thanks
also are you sure that the suggested arrow notation is right?
| }); | ||
|
|
||
| dialog.closeOnBack(); | ||
| dialog.closeOnBack(dialog::hide); |
There was a problem hiding this comment.
Why did you change this? Closing hides the dialog, you're just making it hide twice.
There was a problem hiding this comment.
oh right yeah I just did a substitution, thanks
fixed in 4a02b42
| } | ||
|
|
||
| if(texture == null){ | ||
| if(texture == null || updated){ |
There was a problem hiding this comment.
I don't see you setting updated to false anywhere, meaning it will update every frame once it's set.
There was a problem hiding this comment.
You set update, not updated, which is very different (and will completely break the block)
There was a problem hiding this comment.
You set
update, notupdated
should be fixed in c4fab2d
3 participants
adds:
read/write to canvases
sense canvas width/height (does not include connections)
also should fix #10905
also patched a leak where the texture doesn't get freed when cancelling / pressing ok with no modification / backing out
If your pull request is not translation or serverlist-related, read the list of requirements below and check each box: