Skip to content
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

Resizing with snap does not update drawable area #126

Open
Martmists-GH opened this issue Aug 7, 2021 · 14 comments
Open

Resizing with snap does not update drawable area #126

Martmists-GH opened this issue Aug 7, 2021 · 14 comments

Comments

@Martmists-GH
Copy link

Martmists-GH commented Aug 7, 2021

I made a sample app using lwjgl as backend that just draws some text in the corner:
Screenshot_20210807_103641
This works fine when manually resizing to any size. However, once I start making it snap to a side, it does not update the surface correctly*:
Screenshot_20210807_103748
It does tell me it resized to fullscreen in the handler callback though.

The code handling resizing is as follows:

// init
glfwSetWindowSizeCallback(window) { w, width, height -> onResize(width, height) }

// resize handler
fun onResize(width: Int, height: Int) {
    // create new backend
    val newBackend = BackendRenderTarget.makeGL(
        width,
        height,
        0,
        8,
        fbId,
        FramebufferFormat.GR_GL_RGBA8
    )
    val newSurface = Surface.makeFromBackendRenderTarget(
        context,
        renderTarget,
        SurfaceOrigin.BOTTOM_LEFT,
        SurfaceColorFormat.RGBA_8888,
        ColorSpace.getSRGB()
    )
    
    // keep old values
    val oldSurface = surface
    val oldBackend = renderTarget

    // set new ones
    // these are fields, used in the render loop to allow swapping the surfaces
    renderTarget = newBackend
    surface = newSurface
    
    // clear old values 
    // done this way to prevent closing while processing, 
    //  though this may still happen and I don't know of a good way to handle it
    oldSurface.close()
    oldBackend.close()

    println("onResize $width $height")
    ResizeEvent.EVENT.invoker().invoke(width, height)
}

*it seems it requires an additional resize to actually trigger the surface reload.

@tonsky
Copy link
Collaborator

tonsky commented Aug 8, 2021

Seems to work for me on macOS. Maybe you need to trigger redraw after resize?

@Martmists-GH
Copy link
Author

How would I do so? I'd assumed the loop calling all the drawing methods would do so already

@tonsky
Copy link
Collaborator

tonsky commented Aug 9, 2021

Can you share the source?

@Martmists-GH
Copy link
Author

Martmists-GH commented Aug 10, 2021

class Window {
    // Skija values
    private lateinit var context: DirectContext
    private lateinit var renderTarget: BackendRenderTarget
    private lateinit var surface: Surface

    // Contains utilities and the canvas
    private lateinit var ctx: RenderContext

    private val window: Long

    // initial values
    private val width = 640
    private val height = 480

    // Seems to be needed?
    private var fbId = 0

    // Low numbers are rendered first
    private val layers = PriorityQueue<Pair<Int, RenderLayer>>(compareBy { it.first })

    init {
        glfwInit()
        glfwWindowHint(GLFW_VISIBLE, GLFW_FALSE)
        glfwWindowHint(GLFW_RESIZABLE, GLFW_TRUE)

        window = glfwCreateWindow(width, height, "Hello Window", NULL, NULL)
        glfwMakeContextCurrent(window)
        glfwSwapInterval(1)
        glfwShowWindow(window)

        // set up callbacks
        glfwSetWindowSizeCallback(window) { w, width, height -> onResize(width, height) }
        glfwSetKeyCallback(window) { w, key, scancode, action, mods -> onKey(key, scancode, action, mods) }
        glfwSetMouseButtonCallback(window) { w, button, action, mods -> onClick(button, action, mods) }
    }

    fun start() {
        GL.createCapabilities()
        context = DirectContext.makeGL()
        fbId = glGetInteger(0x8CA6) // GL_FRAMEBUFFER_BINDING

        renderTarget = BackendRenderTarget.makeGL(
            width,
            height,
            0,
            8,
            fbId,
            FramebufferFormat.GR_GL_RGBA8
        )

        surface = Surface.makeFromBackendRenderTarget(
            context,
            renderTarget,
            SurfaceOrigin.BOTTOM_LEFT,
            SurfaceColorFormat.RGBA_8888,
            ColorSpace.getSRGB()
        )

        ctx = RenderContext(surface.canvas)

        // Render loop
        while (!glfwWindowShouldClose(window)) {
            surface.canvas.clear(0xFF000000.toInt() + ctx.theme.BACKGROUND)

            layers.forEach {
                it.second.render(ctx)
            }

            context.flush()
            glfwSwapBuffers(window)
            glfwPollEvents()
        }

        surface.close()
        renderTarget.close()
        context.close()

        glfwFreeCallbacks(window);
        glfwDestroyWindow(window);

        glfwTerminate();
        glfwSetErrorCallback(null)?.free();
    }

    fun addLayer(priority: Int, layer: RenderLayer) {
        layers.add(Pair(priority, layer))
    }

    private fun onResize(width: Int, height: Int) {
        // Create new backend
        val newBackend = BackendRenderTarget.makeGL(
            width,
            height,
            0,
            8,
            fbId,
            FramebufferFormat.GR_GL_RGBA8
        )
        val newSurface = Surface.makeFromBackendRenderTarget(
            context,
            renderTarget,
            SurfaceOrigin.BOTTOM_LEFT,
            SurfaceColorFormat.RGBA_8888,
            ColorSpace.getSRGB()
        )

        // Swap (should reduce issues from using closed backend, assuming this runs on a separate thread)
        val oldSurface = surface
        val oldBackend = renderTarget
        val oldCtx = ctx
        renderTarget = newBackend
        surface = newSurface
        ctx = RenderContext(surface.canvas)

        // Close old backend
        oldSurface.close()
        oldBackend.close()
        oldCtx.textRenderer.close()

        println("onResize $width $height")
        ResizeEvent.EVENT.invoker().invoke(width, height)
    }

    private fun onKey(key: Int, scancode: Int, action: Int, mods: Int) {
        println("onKey $key $scancode $action $mods")
        KeyEvent.EVENT.invoker().invoke(key, scancode, action, mods)
    }

    private fun onClick(button: Int, action: Int, mods: Int) {
        println("onClick $button $action $mods")
        // TODO: Event
    }
}

it clearly logs onResize 1920 1080 when I drag it to the top of my screen to make it full-screen, but the text remains in the middle.

@Martmists-GH
Copy link
Author

I've been able to confirm this affects multiple devices and multiple OSes.
Based on where the contents appear when resizing, I'm guessing the internal buffer does not have the correct size and/or downscales the output to match the previous known backend size, though I have no real proof of this.

@tonsky
Copy link
Collaborator

tonsky commented Aug 15, 2021

Have you tried adding draw to the end of resize handler? That’s what I am doing in LWJGL example, and I don’t see any artefacts (apart from resize being late for ~1 frame, but that’s expected from GLFW):

updateDimensions();
initSkia();
draw();

What OSes have you tested this on?

Also, I don’t think I understad your issue completely. What exactly are you doing (step by step), and what are you seeing? What do you mean by “snap to a side”? When you say “it requires an additional resize to actually trigger the surface reload” do you mean it renders at the wrong size permanently or just for 1 frame?

@Martmists-GH
Copy link
Author

Have you tried adding draw to the end of resize handler? That’s what I am doing in LWJGL example, and I don’t see any artefacts (apart from resize being late for ~1 frame, but that’s expected from GLFW):

Not yet, I'll give that a try.

To be precise, I've tested this on Arch Linux in KDE and on Windows 10. With snapping I mean when you drag a window to a side or corner, and the WM resizes it to full/half/quarter your screen size. The additional resize means resizing the window through normal means afterwards, though this seems to always have the previous window size as rendered size.


Adding a call to draw() at the end of my onResize function (different project, but exact same setup for rendering) does not seem to change anything. One thing I could think of is the value of dpi in that example, though I have no clue what this value does or means, especially since I try to resize to the new window size either way.
Screenshot_20210815_210010

@Martmists-GH
Copy link
Author

I recorded my screen to show how I can reproduce the issue

2021-08-15_21-05-16.mp4

@Martmists-GH
Copy link
Author

Taking screenshots from the above video and lining them up, you can see what I meant with "match the previous known backend size"; the size of the new buffer (left side to where it cuts off) is exactly equal to the previous size of the window

lined_up

@tonsky
Copy link
Collaborator

tonsky commented Aug 18, 2021

I checked with our LWJGL example (which is based on GLFW) and Ubuntu 20.04, seems to work fine. Not sure what’s going on in your case.

Are you sure you get correct width/height in onResize callback? Have you tried moving oldSurface.close()/oldBackend.close() before creating new ones?

@tonsky
Copy link
Collaborator

tonsky commented Aug 18, 2021

Can you check too, btw? Clone this and run ./examples/lwjgl/script/run.py --skija-version 0.93.1. Does it exibit the same behavior?

@Martmists-GH
Copy link
Author

Compiling 55 java files to target/classes
error: release version 16 not supported
Usage: javac <options> <source files>
use --help for a list of possible options
Traceback (most recent call last):
  File "/home/mart/temp/skija/./examples/lwjgl/script/run.py", line 57, in <module>
    sys.exit(main())
  File "/home/mart/temp/skija/./examples/lwjgl/script/run.py", line 40, in main
    common.javac(sources, 'target/classes', classpath = classpath, release = '16')
  File "/home/mart/temp/skija/script/common.py", line 61, in javac
    check_call([
  File "/home/mart/temp/skija/script/common.py", line 23, in check_call
    res = subprocess.check_call(args, **kwargs)
  File "/usr/lib/python3.9/subprocess.py", line 373, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['javac', '-encoding', 'UTF8', '--release', '16', '--class-path', '/home/mart/.m2/repository/org/projectlombok/lombok/1.18.20/lombok-1.18.20.jar:/home/mart/.m2/repository/org/lwjgl/lwjgl/3.2.3/lwjgl-3.2.3.jar:/home/mart/.m2/repository/org/lwjgl/lwjgl-glfw/3.2.3/lwjgl-glfw-3.2.3.jar:/home/mart/.m2/repository/org/lwjgl/lwjgl-opengl/3.2.3/lwjgl-opengl-3.2.3.jar:/home/mart/.m2/repository/org/lwjgl/lwjgl/3.2.3/lwjgl-3.2.3-natives-linux.jar:/home/mart/.m2/repository/org/lwjgl/lwjgl-glfw/3.2.3/lwjgl-glfw-3.2.3-natives-linux.jar:/home/mart/.m2/repository/org/lwjgl/lwjgl-opengl/3.2.3/lwjgl-opengl-3.2.3-natives-linux.jar:/home/mart/.m2/repository/org/jetbrains/skija/skija-shared/0.93.1/skija-shared-0.93.1.jar:/home/mart/.m2/repository/org/jetbrains/skija/skija-linux/0.93.1/skija-linux-0.93.1.jar:target/classes', '-d', 'target/classes', 'src/Main.java', '../scenes/src/BitmapScene.java', '../scenes/src/RunIteratorScene.java', '../scenes/src/ShadowsScene.java', '../scenes/src/DebugTextRun.java', '../scenes/src/ImageCodecsScene.java', '../scenes/src/EmptyScene.java', '../scenes/src/WatchesScene.java', '../scenes/src/ShapersScene.java', '../scenes/src/ImageBenchScene.java', '../scenes/src/FontScene.java', '../scenes/src/Scenes.java', '../scenes/src/SquaresScene.java', '../scenes/src/Pair.java', '../scenes/src/ParagraphScene.java', '../scenes/src/SVGScalingScene.java', '../scenes/src/ColorFiltersScene.java', '../scenes/src/RuntimeEffectScene.java', '../scenes/src/HUD.java', '../scenes/src/DrawableScene.java', '../scenes/src/TextBlobScene.java', '../scenes/src/PathEffectsScene.java', '../scenes/src/ShadersScene.java', '../scenes/src/BlendsScene.java', '../scenes/src/SwingScene.java', '../scenes/src/WallOfTextScene.java', '../scenes/src/SkottieScene.java', '../scenes/src/SVGScene.java', '../scenes/src/PythagorasScene.java', '../scenes/src/TextLineDecorationsScene.java', '../scenes/src/TextLineScene.java', '../scenes/src/PixelGridScene.java', '../scenes/src/RunHandlerScene.java', '../scenes/src/GeometryScene.java', '../scenes/src/ImagesScene.java', '../scenes/src/ParagraphStyleScene.java', '../scenes/src/PathsScene.java', '../scenes/src/BitmapImageScene.java', '../scenes/src/CodecScene.java', '../scenes/src/DecorationsBenchScene.java', '../scenes/src/ShadowUtilsScene.java', '../scenes/src/FontRenderingScene.java', '../scenes/src/MaskFiltersScene.java', '../scenes/src/ImageFiltersScene.java', '../scenes/src/FontSizeScene.java', '../scenes/src/Scene.java', '../scenes/src/TextShapeBenchScene.java', '../scenes/src/FigmaScene.java', '../scenes/src/BreakIteratorScene.java', '../scenes/src/MatrixScene.java', '../scenes/src/TextStyleScene.java', '../scenes/src/DebugTextBlobHandler.java', '../scenes/src/ParagraphMetricsScene.java', '../scenes/src/PictureRecorderScene.java', '../scenes/src/FontVariationsScene.java']' returned non-zero exit status 2.

looks like it fails to compile when running that command with java 11 set, and with java 16:

Compiling 55 java files to target/classes
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
Exception in thread "main" java.lang.AssertionError: Horizontal dpi=1.0, vertical dpi=1.0026315
	at org.jetbrains.skija.examples.lwjgl.Window.updateDimensions(Main.java:77)
	at org.jetbrains.skija.examples.lwjgl.Window.createWindow(Main.java:100)
	at org.jetbrains.skija.examples.lwjgl.Window.run(Main.java:60)
	at org.jetbrains.skija.examples.lwjgl.Main.main(Main.java:32)
Traceback (most recent call last):
  File "/home/mart/temp/skija/./examples/lwjgl/script/run.py", line 57, in <module>
    sys.exit(main())
  File "/home/mart/temp/skija/./examples/lwjgl/script/run.py", line 43, in main
    common.check_call([
  File "/home/mart/temp/skija/script/common.py", line 23, in check_call
    res = subprocess.check_call(args, **kwargs)
  File "/usr/lib/python3.9/subprocess.py", line 373, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['java', '--class-path', 'target/classes:/home/mart/.m2/repository/org/projectlombok/lombok/1.18.20/lombok-1.18.20.jar:/home/mart/.m2/repository/org/lwjgl/lwjgl/3.2.3/lwjgl-3.2.3.jar:/home/mart/.m2/repository/org/lwjgl/lwjgl-glfw/3.2.3/lwjgl-glfw-3.2.3.jar:/home/mart/.m2/repository/org/lwjgl/lwjgl-opengl/3.2.3/lwjgl-opengl-3.2.3.jar:/home/mart/.m2/repository/org/lwjgl/lwjgl/3.2.3/lwjgl-3.2.3-natives-linux.jar:/home/mart/.m2/repository/org/lwjgl/lwjgl-glfw/3.2.3/lwjgl-glfw-3.2.3-natives-linux.jar:/home/mart/.m2/repository/org/lwjgl/lwjgl-opengl/3.2.3/lwjgl-opengl-3.2.3-natives-linux.jar:/home/mart/.m2/repository/org/jetbrains/skija/skija-shared/0.93.1/skija-shared-0.93.1.jar:/home/mart/.m2/repository/org/jetbrains/skija/skija-linux/0.93.1/skija-linux-0.93.1.jar', '-Djava.awt.headless=true', '-enableassertions', '-enablesystemassertions', '-Xcheck:jni', '-Dskija.logLevel=DEBUG', 'org.jetbrains.skija.examples.lwjgl.Main']' returned non-zero exit status 1.

Note that in both cases nothing visually appeared.

@Martmists-GH
Copy link
Author

Are you sure you get correct width/height in onResize callback?

Yes

Have you tried moving oldSurface.close()/oldBackend.close() before creating new ones?

Yes, that still has the same issue.

@tonsky
Copy link
Collaborator

tonsky commented Aug 19, 2021

Remove assert from updateDimensions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants