search

Gamua / Starling-Framework

The Cross Platform Game Engine
http://www.starling-framework.org
Other
2.82k stars 821 forks source link

Android textures getting cropped - workaround? #1012

Closed tconkling closed 5 years ago

tconkling commented 6 years ago

Preamble!

I'm being bitten by a very old AIR-on-Android issue, described in detail on the Starling forum: https://forum.starling-framework.org/topic/textures-partially-uploaded-to-gpu-original-bitmapdata-is-perfect

In short, on Android, textures will occasionally be "cropped" when uploaded to the GPU: all texels below a certain line in the texture will render as invisible. There's no programmer-observable rhyme or reason to when this occurs, and quitting and restarting the game will frequently solve the problem. Here's an example from my game. I've circled the areas subtextures are cropped or completely invisible:

broken

Here's what the screen should look like:

working

Folks have been reporting this issue way back since AIR 19, and it's still recurring as of AIR 27. Adobe's been notified of it several times. The original Adobe tracker bug was seemingly lost when Adobe updated their bug tracking; a new bug is here: https://tracker.adobe.com/#/view/AIR-4198475

Actually getting to the point!

Adobe seems to have like 1/2 an engineer on AIR these days, so the chances of them fixing this bug are slim. My game is blocked until this is fixed or I can work around it.

Anecdotally, it sounds like there's a potential workaround: after creating a ConcreteTexture on Android, immediately draw that texture to a render-texture and compare the results with the source image that was uploaded to the GPU. If there's a mismatch, recreate the texture.

I'd love for this workaround to be baked into Starling itself (presumably via an opt-in flag that client code can set on Starling during initialization, e.g. Starling.validateTextureAfterUpload or something). This would obviously slow texture loading (hence the opt-in), but better a slightly slower load than a broken game.

@PrimaryFeather, does this sound reasonable? I'm happy to take a stab at this and make a PR if it seems like something you'd want in Starling. It looks like ConcretePotTexture.uploadBitmapData and ConcreteRectangleTexture.uploadBitmapData could check a Starling flag and perform this validation in a loop (with some low number of maximum iterations), delaying any TextureReady event until successful?

PrimaryFeather commented 6 years ago

Damn, so this one has bitten you, too. Sorry to hear about those troubles! 😣

You're right, a workaround would be a huge help, and I'd be willing (while holding my nose) to add something like that to Starling — however, how do you plan to check what's inside the texture? There is no way to read pixels from the texture, after all. Well, we could draw it to the back buffer and use "context.drawToBitmapData", but that might lead to flickering on the back buffer — at least I sometimes noticed this in the past. Maybe it's worth a shot; I don't see an easier way.

Nevertheless, I'd like to make another attempt to get this to Adobe, in parallel with you making the pull request. I've got a few questions for you to get this started.

Thanks a lot in advance, Tim!

tconkling commented 6 years ago

Yeah, my plan is to do something along these lines (using the technique described by FatRedBird, in this thread: https://github.com/Gamua/Adobe-Runtime-Support/issues/25)

        var maskColor:uint = 0xFF000000;
        var emptyColor:uint = 0x00000000;

        var tex :Texture = Texture.fromBitmapData(sourceBitmap, false, false, _scale);

        if (_validateAfterCreation) {
            var bounds1:Rectangle = sourceBitmap.getColorBoundsRect(maskColor, emptyColor, false);
            var renderBitmap :BitmapData = new BitmapData(sourceBitmap.width, sourceBitmap.height, true);
            var valid :Boolean;
            for (var ii :int = 0; ii < 3 && !valid; ++ii) {
                var image :Image = new Image(tex);
                image.drawToBitmapData(renderBitmap);
                var bounds2:Rectangle = renderBitmap.getColorBoundsRect(maskColor, emptyColor, false);
                valid = bounds1.equals(bounds2);
                if (!valid) {
                    tex = Texture.fromBitmapData(sourceBitmap, false, false, _scale);
                }
            }
        }

        if (!valid) { throw an error or log a warning; }

(Edited to add: I haven't tested the above code yet, but I'll be integrating it into my game today. I'll let you know if it reliably detects & fixes the bug.)

tconkling commented 6 years ago

An update, after a day spent working on this:

I ended up using this (really crappy) code for texture validation:

public static function createFromBitmap (sourceBitmap :BitmapData, scale :Number, validate :Boolean, name :String = null) :Texture {
    if (!validate) {
        return Texture.fromBitmapData(sourceBitmap, false, false, scale);
    }

    const MASK_COLOR :uint = 0xFF000000;
    const EMPTY_COLOR :uint = 0x00000000;

//        var srcBounds :Rectangle = sourceBitmap.getColorBoundsRect(MASK_COLOR, EMPTY_COLOR, false);
    var renderBitmap :BitmapData = new BitmapData(sourceBitmap.width, sourceBitmap.height, true);
    var tex :Texture = null;
    var valid :Boolean = false;
    for (var ii :int = 0; ii < 3 && !valid; ++ii) {
        if (tex != null) {
            tex.dispose();
        }
        tex = Texture.fromBitmapData(sourceBitmap, false, false, scale);
        var image :Image = new Image(tex);
        image.drawToBitmapData(renderBitmap, EMPTY_COLOR, 0.0);

//            var renderBounds :Rectangle = renderBitmap.getColorBoundsRect(MASK_COLOR, EMPTY_COLOR, false);
//            valid = renderBounds.equals(srcBounds);
        valid = compareBitmaps(sourceBitmap, renderBitmap);
        if (!valid) {
            log.warning("Texture doesn't match source bitmap", "name", name, "tries", ii + 1);
        }
    }

    renderBitmap.dispose();

    return tex;
}

private static function compareBitmaps (src :BitmapData, dst :BitmapData) :Boolean {
    if (src.width != dst.width || src.height != dst.height) {
        return false;
    }

    for (var yy :uint = 0; yy < src.height; ++yy) {
        for (var xx :uint = 0; xx < src.width; ++xx) {
            var srcPx :uint = src.getPixel32(xx, yy);
            var dstPx :uint = src.getPixel32(xx, yy);
            if (Color.getAlpha(srcPx) != 0 && srcPx != dstPx) {
                return false;
            }
        }
    }

    return true;
}

(I'm comparing the two bitmaps pixel by pixel, rather than using the BitmapData.getColorBoundsRect technique, because I cannot get getColorBoundsRect to behave as expected. I'm using it to find the bounds of the non-transparent pixels in both bitmaps, but I'm getting unequal bounds results with this code even on platforms that load textures properly. So either getColorBoundsRect is broken or my understanding of it is.)

BUT: with this code in place, I never trigger the "Texture doesn't match source bitmap" warning. A true heisenbug. Does the drawToBitmapData possibly prevent the error from occurring? Is there a race condition in AIR texture creation/upload on Android? Did I just get unlucky in the dozen or so times I ran the code with this "fix" in place? Who knows. But removing the above code causes the issue to recur.

PrimaryFeather commented 6 years ago

Thanks a lot for all the additional information, Tim, and for trying out that workaround code!

That said, if the existing test case that's attached to the Adobe Tracker bug isn't enough for their purposes [...]

Which test case do you mean? Unfortunately, there is no test case attached — despite me constantly talking about it, no one ever posted an actual sample project that's confirmed to reproduce the issue. If you'd be able to do that, you'd be my personal (Anti?) hero! :wink:

I must admit I only just read about that getColorBoundsRect method — somehow, I never noticed it before. So I can't say if you're using it the right way (although from the documentation, I would also have expected the code you wrote to work).

However, if this continues to not pop up while that code is included, this might be a great pointer as to the internal reason for the bug. I agree: this sounds like a race condition triggered by the texture creation code. Something inside image.drawToBitmapData must prevent it from happening.

tconkling commented 6 years ago

You're right. I could've sworn there was sample code that reproduced the bug on affected devices; maybe in the original bug that was lost during the bugbase switchover? Regardless, I'll work on creating a test case today.

tconkling commented 6 years ago

I've created a small project that reliably reproduces the bug on my tablet. It's here: https://github.com/tconkling/AIRAndroidCroppedTextures. There's a video of my repro'ing the bug here: https://www.youtube.com/watch?v=v0M1Z00_rUU&t=27s. (This is also linked in the Github project.)

PrimaryFeather commented 6 years ago

Thanks a lot for the big efforts on getting this reproduced with sample code and video! That will be a HUGE help to get this looked into and hopefully fixed. I'll post the links to those resources to all relevant reports and will give Adobe another heads-up. Depending on the feedback I'm getting, I'll also decide on including that workaround code in Starling.

I'll keep you updated, Tim!

tconkling commented 6 years ago

Great, thanks Daniel

tconkling commented 6 years ago

@PrimaryFeather - Adobe's updated the bug to say that they can repro it and are working on a fix.

In the meantime - as my game is due to ship in the early new year and Adobe AIR releases are months apart - I need to make do with AIR 28 (which doesn't fix the bug). It turns out my workaround, above, was broken in a boneheaded way: in compareBitmaps, I'm assigning dstPx from the source bitmap, so the comparison doesn't do anything.

I've fixed that, but I'm having a couple issues with DisplayObject.drawToBitmapData: (You may like a separate bug for these issues, which I'm happy to create - let me know)

var textureImage :Image = new Image(texture);
textureImage.scale = texture.scale / Starling.current.contentScaleFactor;
var bitmap :BitmapData = textureImage.drawToBitmapData();

(I think this second issue can be overcome for the purposes of working around this texture bug, as we only need to compare the bottom-most row of non-transparent pixels in the source bitmap to the same row in the rendered bitmap - bugged textures are always vertically cropped at an arbitrary offset from their bottom.)

PrimaryFeather commented 6 years ago

@PrimaryFeather - Adobe's updated the bug to say that they can repro it and are working on a fix.

Hurray, that's great news! Let's hope they get to the root of this issue.

Texture scaling and stage contentScaleFactor seem to be improperly handled.

Could you send me a complete example of the problem you're seeing? I can't quite get to reproduce it. Beware, though, that the "BitmapData" itself does not have any scale factor, so you need to take the different coordinate system into account when accessing it — or assign the correct scale factor when you convert it back to a texture.

The resulting texture will be cropped if it's bigger than the context 3D back buffer.

Yes, that's currently a limitation. Since stage3D only supports reading back pixels from the back buffer (not, say, a render texture), the drawToBitmapData method draws to the back buffer as it's currently configured for the application. I'll have to try if it's possible to temporarily resize it beyond the bounds of the current application.

In any case, as long as there is no proper workaround, I will add this information to the docs. Thanks for the reminder!!

tconkling commented 6 years ago

@PrimaryFeather, re: complete example -- do you mean a working application, or simply a screenshot of what I'm seeing alongside a code snippet?

Also, it turns out - for my test devices at least - that it's the bitmap loading that is failing, and not texture creation. I wrote more about it here, https://github.com/Gamua/Adobe-Runtime-Support/issues/25. So my findings are significantly different from @FatRedBird's (whose results I couldn't replicate), and I wonder if he was simply mistaken or if there are in fact multiple bugs here?

PrimaryFeather commented 6 years ago

do you mean a working application, or simply a screenshot of what I'm seeing alongside a code snippet?

Screenshot (or explanation text) + code snippet would be totally sufficient!

Also, it turns out - for my test devices at least - that it's the bitmap loading that is failing, and not texture creation.

Oh wow, so it is the bitmap loading that's the problem. Some users in the old forum thread suspected something like that, but we never managed to proof this via an example. You're right, this could mean that we're looking at two different things here. On the other hand, the phenomenon is so similar ... hm.

PrimaryFeather commented 6 years ago

BTW, I'm happy to hear you found a workaround that sounds feasible for the time being! Hopefully it finally gets fixed at its root — thanks to you, it's now possible — but it's good that this isn't a show stopper any longer.

tconkling commented 6 years ago

Sure - here's a function that takes a texture, draws it onscreen in an untransformed Image, and then renders that Image to a bitmap with the same dimensions as the texture, creates a new texture from the bitmap, and draws that to the screen next to the original image:

private function drawTexture (tex :Texture) :void {
    // draw the texture
    var texImage :Image = new Image(tex);

    // render the texture to a bitmap, turn that into a new texture, and draw that
    var renderBitmap :BitmapData = new BitmapData(tex.nativeWidth, tex.nativeHeight, true);
    texImage.drawToBitmapData(renderBitmap);
    var renderTex :Texture = Texture.fromBitmapData(renderBitmap, false, false, tex.scale);
    var bitmapImage :Image = new Image(renderTex);

    // show the two images onscreen (snip)
}

Here are the results with a sample texture from my game. My textures all have a scale of 2, and at the game's default resolution, 1024x768, Starling.contentScaleFactor is also 2 -- the texture scale and contentScaleFactor are equal. Here's what that looks like (original texture on the left, rendered-to-bitmap texture on the right):

screen shot 2017-12-21 at 5 09 45 pm

When I fullscreen my game on my 2560x1440 monitor, the game's updated viewPort results in a Starling.contentScaleFactor of 4. Here's the results of running the same function when contentScaleFactor is larger than texture scale (this screenshot and its contents are larger than the previous, but this is simply because the game is scaled up to account for the big jump in resolution):

screen shot 2017-12-21 at 5 09 56 pm

(If the source texture has a larger scale than Starling.contentScaleFactor -- if I were to run the game at a super tiny resolution -- that texture will be shrunken in the resulting bitmap.)

Client code can fix this by scaling the source Image before calling drawToBitmapData, like so:

texImage.scale = (tex.scale / Starling.current.contentScaleFactor);
texImage.drawToBitmapData(renderBitmap);

It's possible that things are working as intended and I simply misunderstood the drawToBitmapData function. My expectations were that an untransformed Image would produce the same result regardless of contentScaleFactor, and I was surprised that the results were essentially resolution-dependent. But I can certainly understand an argument for the existing behavior as well - it's probably what devs with in-game screenshotting functionality expect, for example.

PrimaryFeather commented 6 years ago

Mhm, as you already guessed, this is actually working as intended, but I can see why this is a little confusing.

I think the problem is this: you're treating drawToBitmapData as if it was a method on Image, not DisplayObject. If it were on Image, I could take the image texture's scale factor into account and create the bitmap data in its optimal size. However, as it's a method on DisplayObject, I don't know anything about a texture. The object could be a Canvas or an untextured Quad, or a Sprite with all different kinds of objects.

Thus, Starling will do the second best thing and use the "dpi" of the stage, if you will. The result is BitmapData that, if converted to a texture, should use Starling's current contentScaleFactor. Your code would look like this:

// draw the texture
var texImage :Image = new Image(tex);

// render the texture to a bitmap, turn that into a new texture, and draw that
var renderBitmap :BitmapData = texImage.drawToBitmapData();
var renderTex :Texture = Texture.fromBitmapData(renderBitmap, false, false, Starling.contentScaleFactor);
var bitmapImage :Image = new Image(renderTex);

That way, the two images will have the same size & resolution.

Again, you already expected that — I just wanted to describe my reasoning in a little more detail so it's comprehensible. In a nutshell, drawToBitmapData is a generic method that must work the same for all display objects, and that's why it behaves as it does. :smile:

ll24llll commented 6 years ago

Hello, I have the same problem, some Textures are partially previewed, cropped or completely invisible. I'm using AssetManager class to get textures from atlas files. So the workaround introduced above doesn't work for me. I don't have access to BitmapData of SubTextures of atlas files.

Is this problem going to get fixed?

PrimaryFeather commented 6 years ago

I recently rewrote the whole AssetManager architecture, so it would be possible to implement Tim's solution via an extension to that.

However, the problem remains that the final row often is empty/transparent, so I'm not sure yet how best to automate detection. I'll try to think of something! @ll24llll, how reliably can you reproduce the problem? If I sent you some test-code, would you be able to verify if it works?

ll24llll commented 6 years ago

Dear Daniel, I'll check out the new AssetManager as soon as possible, thanks.

Unfortunately I can not reproduce the problem myself, but some of my clients are sending me screen captures like this every now and then: picture link

By implementing Tim's solution and tracking it, I'll be aware of that how much and in which situations the problem is occurring.

tconkling commented 6 years ago

AIR 29 claims to have fixed this issue - have you tried the latest version?

On Wed, Apr 11, 2018 at 11:14 PM, ll24llll notifications@github.com wrote:

Dear Daniel, I'll check out the new AssetManager as soon as possible, thanks.

Unfortunately I can not reproduce the problem myself, but some of my clients are sending me screen captures like this every now and then: picture link http://s9.picofile.com/file/8323517618/tank_texture_bug.jpg

By implementing Tim's solution and tracking it, I'll be aware of that how much and in which situations the problem is occurring.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Gamua/Starling-Framework/issues/1012#issuecomment-380690592, or mute the thread https://github.com/notifications/unsubscribe-auth/AArRnkbQP8F3MaHI830-pfHKOdrbSzHXks5tnvCtgaJpZM4Q_3HU .

ll24llll commented 6 years ago

Dear Tim, I implemented AIR29 the day before yesterday and I've not published an update thereafter. I'll let you know if the problem persists.

On Sun, Apr 15, 2018 at 3:00 AM, Tim Conkling notifications@github.com wrote:

AIR 29 claims to have fixed this issue - have you tried the latest version?

On Wed, Apr 11, 2018 at 11:14 PM, ll24llll notifications@github.com wrote:

Dear Daniel, I'll check out the new AssetManager as soon as possible, thanks.

Unfortunately I can not reproduce the problem myself, but some of my clients are sending me screen captures like this every now and then: picture link http://s9.picofile.com/file/8323517618/tank_texture_bug.jpg

By implementing Tim's solution and tracking it, I'll be aware of that how much and in which situations the problem is occurring.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Gamua/Starling-Framework/issues/ 1012#issuecomment-380690592, or mute the thread https://github.com/notifications/unsubscribe-auth/AArRnkbQP8F3MaHI830- pfHKOdrbSzHXks5tnvCtgaJpZM4Q_3HU .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Gamua/Starling-Framework/issues/1012#issuecomment-381365133, or mute the thread https://github.com/notifications/unsubscribe-auth/ABUILjTWIawKRIAflpR5apWnyV01jsolks5tonh4gaJpZM4Q_3HU .

AmritaGangwani commented 6 years ago

The issue has been addressed with AIR 29.

PrimaryFeather commented 6 years ago

@ll24llll: could you verify if this issue is solved for you? Then I can close this issue.

PrimaryFeather commented 5 years ago

I haven't seen this problem reported since that mentioned AIR update, so I'm supposing this is fixed! I'll close the issue now, but if anyone reports differently, we can open it up again anytime.