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

Optimization: double vertex and index buffers usage during Stage.drawToBitmapData #638

Closed
wants to merge 1 commit into from

Conversation

stepan-beresnev
Copy link

In Facebook web games to show html (FB popups, for example) over flash with wmode direct we are doing stage screenshots, jpeg them, encode into base64 string and pass it to javascript, which in turn hides flash and replaces it with image received. I think, that’s usual approach for Starling Facebook games.
The problem is that some of our users are getting errors like that sometimes:
ArgumentError: Error #3672: Buffer creation failed. Internal error.
at flash.display3D::Context3D/createVertexBuffer()
at starling.display::QuadBatch/createBuffers()
at starling.display::QuadBatch/syncBuffers()
at starling.display::QuadBatch/renderCustom()
...
which are thrown from random QuadBatches. And for some reason before this change quite a lot of such errors were thrown from drawToBitmapData function. It means, that rendering into bitmap works much worse than usual render for some reason, taking into account, that it happens let’s say once per 10 minutes, and render happens every frame. Probably that’s because buffers are reused in usual rendering.
In this change I reused Starling’s render support object.
Documentation states: “Buffer Creation Failed: if the VertexBuffer3D object could not be created by the rendering context (but additional information about the reason is not available).” So it’s not completely clear for me, why this error happens, but after this change it’s very likely, that new vertex and index buffers won’t be created at all during stage drawing into bitmap. It means twice less GPU memory usage for vertex and index buffers and speed up of the function as there is no need to create them.
Also current implementation doesn’t dispose RenderSupport object created, which can cause GPU memory leaks. And it’s not needed after the change.
The change also includes context validity check, if context isn’t valid, function won’t crash, but will return empty bitmap.
Also it will try to reconfigure back buffer, if it’s needed.
The thing, I’m not sure about is using mClippedViewPort for setProjectionMatrix’s parameters instead of stage, this way it should render only visible stage part, if I’m not mistaken. It seems to me, that it’s more correct behaviour, but I’m not sure.

@PrimaryFeather
Copy link
Contributor

Thanks a lot for the pull request!

It's interesting that the error happens especially when you're using that method — but, as you say: since it always has to create new QuadBatches internally, it's plausible that this is the cause of running out of resources.

When looking at the original code, however, I noticed that it was lacking a "dispose" call on the new "RenderSupport" instance. So those buffers were never really released! Thus, I strongly suspect that we can get away with it by simply adding that call.

I'll make that change and commit it — and then, I would ask you if you could test this change with your game. Do you think this is possible? Thanks a lot in advance!

@PrimaryFeather
Copy link
Contributor

I committed the mentioned dispose-change here: c444ff1

@stepan-beresnev
Copy link
Author

Sorry, can't test it. The game has a lot of users, that's the reason we can collect errors, which are difficult to reproduce ourselves, and that's the reason I can't test the code, which might not work. But honestly I'd like to test, if I could :)
Dispose should help a bit, but I'd prefer to recreate buffers as less as possible, because we are still getting these errors even in usual rendering after the change I did in the pull.
And reusing buffers also makes screenshots faster.
And context validity test, it's the same issue as in 627.

@PrimaryFeather
Copy link
Contributor

No worries, I can understand that testing this on a live game with lots of users is not easy. ;-)

@vpmedia
Copy link
Contributor

vpmedia commented Nov 5, 2014

Hi,
Maybe not the best solution but i'm using the following snippet to generate screenshots. Because the RenderSupport is static and only initialized once, it does not leak, I think/hope : )

private static var support:RenderSupport;
public static function snapshot(offsetX:int = 0, offsetY:int = 0):BitmapData {
    // get screen size
    const w:uint = Starling.current.stage.stageWidth;
    const h:uint = Starling.current.stage.stageHeight;
    // create or get render support
    if (!support)
        support = new RenderSupport();
    else
        support.nextFrame();
    support.clear(0x0, 1.0);
    support.setProjectionMatrix(offsetX, offsetY, w, h);
    // render screen
    Starling.current.stage.render(support, 1);
    support.finishQuadBatch();
    // render screen
    const result:BitmapData = new BitmapData(w, h, true);
    Starling.context.drawToBitmapData(result);
    // return result
    return result;
}

@vpmedia
Copy link
Contributor

vpmedia commented Nov 5, 2014

Sorry, I see now that something similar has been added to Stage.as

@ivan-shaban
Copy link

That would be good if you improve function to take screenshot from concrete display object (optional).
Also is it works fine on mobile devices where Starling.contentScaleFactor may be different?

@PrimaryFeather
Copy link
Contributor

In Starling 2, the drawToBitmapData method uses the same RenderUtil (now: Painter) instance as is used for all the rest of the rendering. That change was in fact partly triggered by this pull request.

So you can regard this as 'merged'! 😄

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

Successfully merging this pull request may close these issues.

4 participants