Gamua / Starling-Framework

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

AssetManager.processXml should remove texture after all bitmap font registered. #997

Closed soimy closed 6 years ago

soimy commented 6 years ago

For now, AssetManager.processXml remove texture immediately after bitmap font registered.

if (texture)
{
    log("Adding bitmap font '" + name + "'");
    TextField.registerCompositor(new BitmapFont(texture, xml), name);
    removeTexture(name, false);

    if (_keepFontXmls) addXml(name, xml);
    else System.disposeXML(xml);
}

When several font file point to the same atlas texture, the second font will fail because the texture is removed. How about collect these texture in an Array and remove them together?

soimy commented 6 years ago

Also, we should use the font face instead of texture name as bitmap font name.

<?xml version="1.0"?>
<font>
    <info face="DIN_CB" type="msdf" size="42" bold="0" italic="0" charset="" unicode="1" stretchH="100" smooth="1" aa="1" padding="0,0,0,0" spacing="0,0"/>
PrimaryFeather commented 6 years ago

Yes, I'll add a list of textures (as in your pull request) or maybe even a way to access all the bitmap fonts via getBitmapFont, etc. That's the proper way to do it.

As for using the font face: actually, that was how it was originally implemented in an older version; but this lead to problems when developers added the same font face multiple times (e.g. as normal and bold variants), and we decided it's simpler to use just the texture name — since that's definitely unique, and easy to change.

soimy commented 6 years ago

if use texture name,it will be impossible to use multiple font packed in one texture. How about font file name? That's unique too.

PrimaryFeather commented 6 years ago

if use texture name,it will be impossible to use multiple font packed in one texture. How about font file name? That's unique too.

Hm, that's a good point. Perhaps the font file name would be preferable, you're right! I'll look into that.

Klug76 commented 6 years ago

Please can you also make protected BitmapFont::parse(data: *) function, instead of private parseFontXml? It will be good to support other formats, e.g. json/amf/...

PrimaryFeather commented 6 years ago

@soimy I decided it's best to stick with the old behavior per default (i.e. using the texture name for font registration), so that I'm not breaking any logic from existing games. However, I added a new property to the AssetManager: registerBitmapFontsWithFontFace — if enabled, the bitmap font is registered with the face attribute found in the font XML file. That makes the AssetManager now fully compatible with your method of combining the glyphs of multiple fonts into one texture. :smile:

@Klug76 You're right, that's an excellent idea (sorry for not having commented on your request earlier!). I just made that change, too!