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

Discussion: Texture Packing and Texture Atlas #620

Closed
Davhed opened this issue Apr 27, 2013 · 56 comments
Closed

Discussion: Texture Packing and Texture Atlas #620

Davhed opened this issue Apr 27, 2013 · 56 comments

Comments

@Davhed
Copy link
Member

Davhed commented Apr 27, 2013

WORKING BRANCH

This post is so that I can layout my idea for how a texture atlas could/should work. Since this is on the challenging side for me I wanted to ensure I got some input from the gurus.

Concept

Texture atlas code has been written a thousand times over, and there is a lot of data that says to keep things simple. I am going to be adapting some code for mobile and Rajawali, the forked source can be found here.

Basically, a new class TexturePacker.java would be created and would handle the bulk of the work, and it would return a new object type TextureAtlas which can hold the data about each "tile" in the atlas as well as the atlas Bitmaps

public TextureAtlas mTextureAtlas = TexturePacker(String atlasName, int atlasWidth, int altasHeight, int padding)

TextureAtlas would contain the altasBitmaps, and a list of file names and coordinates(X,Y,W,H) for each texture packed into the atlas.

I am thinking there should be a getter to retrieve the image coordinates needed for a given object. Something like -- TextureAtlas.getCoords(String textureFileName). We then need a way to translate those coordinates into UVs.. this part is still hazy for me. Once the developer has the UVs they can simply get the appropriate atlas Bitmap by using another getter -- `TextureAtlas.getAtlas(String textureFileName).

Scope

Loading

  1. Designate folder and populate with textures
    • Possibly drawable-nodpi, and append a prefix/suffix to each texture
    • Custom folder accessible at runtime (if possible I would need some suggestions)
  2. Retrieve files from target folder
    • Capture file name, width, and height of each

Packing

  1. Developer defines the size of the atlas on construction
  2. Utilize this algorithm for simple and efficient packing
    • Store coordinates of each tile as it is placed with reference to the original file name
  3. Collect loaded Bitmaps into a Canvas
    • Draw as many Bitmaps as needed to accomodate all files in the target folder
    • Support compression of final Bitmaps??
  4. Return a TextureAtlas will all collected data and atlas Bitmaps

Retrieval

  1. Getters will allow retrieval of all, or specific data within the TextureAtlas
  2. Return the correct atlas bitmap based on file name
  3. Return correct coordinates based on file name
    • Convert texture coordinates to UVs (need some direction here)

Uncertainties

Compression support is possible, but I'm not sure it make sense. It seems that a bitmap could be returned and the "on-the-fly" compression could be implemented is the developer so desired. I don't think that this should support compressed textures on input.

Is there a better way to collect the images besides a Canvas? Any prodding in the right direction is super helpful.

Please give me whatever feedback you can, I will do my best to learn whatever I need to to get this done properly. Thanks!!

@ToxicBakery
Copy link
Member

Yay, this will make custom fonts easy =)

@Davhed
Copy link
Member Author

Davhed commented Apr 27, 2013

In theory it should increase frame rate for texture heavy scenes too.

Maybe I should include my textToBitmap code as a utility related to this. It takes any input string, uses any font from resource, and returns a PoT bitmap that fits the string.... hmmm

@jwoolston
Copy link
Member

👍 for the text support if you're willing to open it up.

@jwoolston
Copy link
Member

@daveHD after thinking about it some I think it makes the most sense to make it an input to enable compression on the fly when the atlas output is added to the texture manager. I'm not sure if that's what you are describing but its how I would do it

@Davhed
Copy link
Member Author

Davhed commented Apr 29, 2013

FWIW I'm @Davhed, not that Davehd guy :P

That makes good sense. Enabling texture compression would be a param in the packing class:

TexturePacker(String atlasName, int atlasWidth, int altasHeight, int padding, <b>boolean useCompression</b>)

Packing would be a one-time process, but TextureAtlas would be a reusable asset. An object will need to be able to access the texture (bitmap or compressed). I was thinking that usage of the atlas would be something like this:

mObject.addTexture(TextureAtlas.getBitmap(fileName)); // alternatively .getCompressedAtlas(fileName)

It will also require modified UVs. Instead of a range of (0,0) to (1,1) it will be a a range of (aTile.x1, aTile.y1) to (aTile.x2, aTile.y2). How to do best handle this conversion is still hazy.

@MasDennis
Copy link
Member

Packing would be a one-time process, but TextureAtlas would be a reusable asset.

This makes me think we should integrate it into the new TextureManager class.
Something along the lines of

// -- create texture atlas
TextureAtlas atlas = mTextureManager.createTextureAtlas(String name, ... other params);
// -- ... do atlas stuff ...

// -- retrieve texture atlas
TextureAtlas atlas = mTextureManager.getTextureAtlas(String name);
// -- ... do atlas stuff ...

// -- create texture
TextureConfig config = new TextureConfig();
config.setAtlas(atlas);
config.setAtlasTextureName(textureName);

mTextureManager.addTexture(config);

This way we can hide a lot of the implementation details from the user.
The TextureAtlas object could hold information about UV coordinate transformation (u translate, v translate, u scale, v scale) which can then be used to update the texture coordinate buffer when mObject.addTexture() is called.

Just thinking out loud here, you're welcome to shoot it down 👍

@jwoolston
Copy link
Member

FWIW I'm @Davhed, not that Davehd guy :P

Sorry, I know the difference. Unfortunately the GitHub Android app does not have pop-up tagging so sometimes my spell correction gets in the way if I don't pay attention.

If we can go with what @MasDennis presented, it would help in integrating to the task queue to make sure nothing funky happened to the developer when trying to create the new atlas.

@Davhed
Copy link
Member Author

Davhed commented Apr 29, 2013

@MasDennis Oh yeah packing via TextureManager is a much better solution, and @jwoolston makes an even more important point since we are moving to a queue.

So, with regards to the UV transformation, the logic in addTexture() would then change, so that if a TextureAtlas is added the TCB would be automagically updated? I like that idea a lot, and it would provide a much more streamlined integration. Any worries about having the packing code within the TextureManager class? It is fairly lengthy

@MasDennis
Copy link
Member

so that if a TextureAtlas is added the TCB would be automagically updated?

Yes, BaseObject3D's addTexture() method would pass the UV transformation information the its Geometry3D member. This member will take care of the transformation and will updated the buffer.

Any worries about having the packing code within the TextureManager class?

The packing code will still be in the TexturePacker class but it will be instantiated and managed by the TextureManager class.

@Davhed
Copy link
Member Author

Davhed commented Apr 29, 2013

Brilliant, that makes good sense. Thanks for the direction help. I'm going to put some ground work for this together over the next couple of days.

@Davhed
Copy link
Member Author

Davhed commented Apr 30, 2013

I have run into a question already, and I am hoping theres a good solution.

As I mentiond before I'd like packing of textures to happen once at the start up of a given app. Packing consistes of four primary stages: Read in image assets, sort by size, draw images to the atlas, return the complete atlas + info.

Currently this is how I am pulling in files:

        /*
         * Retrieve asset names from "assets/atlas" 
         */
        AssetManager am = context.getAssets();
        String[] fileList;
        try {
            fileList = am.list("atlas");
        } catch (Exception e) {
            throw new RuntimeException(e);
        }

        /*
         * Pull atlas assets and convert to bitmaps for processing
         */
        Bitmap[] atlasTiles = new Bitmap[fileList.length];

        for(int i = 0; i < fileList.length; i ++){
            try {
                System.out.println(fileList[i]);
                InputStream is = context.getAssets().open("atlas/"+fileList[i]);
                atlasTiles[i] = BitmapFactory.decodeStream(is);
            } catch (Exception e) {
                  throw new RuntimeException(e);
            }
        }

As you can see I have opted to put a subdirectory in "assets", and I am pulling those files via InputStream. I then immediately convert the stream to a Bitmap because I need the image size for sorting. Ideally I'd like to get the image sizes before making the files into Bitmaps since every Bitmap will reside in RAM making the packing procedure very bloated and likely to fail on mobile devices. I know that javax has an ImageIO class that helps with this... is there an equivalent for Android? Or a better way to approach this part?

@Davhed
Copy link
Member Author

Davhed commented Apr 30, 2013

Actually I think I might have found what I needed in BitmapFactory

public boolean inJustDecodeBounds

Added in API level 1
If set to true, the decoder will return null (no bitmap), but the out... fields will still be set, allowing the caller to query the bitmap without having to allocate the memory for its pixels.

@jwoolston
Copy link
Member

@Davhed Sorry, I was out at lunch. Yes that is the ideal solution.

@Davhed
Copy link
Member Author

Davhed commented Apr 30, 2013

@jwoolston Hey no worries thank you for the confirmation!

@jwoolston
Copy link
Member

@Davhed I just remembered this annoying tidbit from Andorid Developers Website: http://developer.android.com/tools/projects/index.html

Library projects cannot include raw assets
The tools do not support the use of raw asset files (saved in the assets/ directory) in a library project. Any asset resources used by an application must be stored in the assets/ directory of the application project itself. However, resource files saved in the res/ directory are supported.

I am not certain how you were utilizing the assets folder, but something to think about before you get too far.

@Davhed
Copy link
Member Author

Davhed commented Apr 30, 2013

@jwoolston What I am thinking is that the RajawaliTemplate project would be updated to have an assets/atlas folder, and the wiki/javadoc would explain to developers that to use the TexturePacker they would have to put their image files in that folder rather than drawable-nodpi as a way to isolate the images to be processed without having to append a prefix or suffix to their filenames. If there is a problem with this method or if there is a better alternative please let me know!

Side note:

I figure that TextureAtlas is just another type of texture and therefore should exist under rajawali.materials, but it seems that TexturePacker should be a util. Sane?

@jwoolston
Copy link
Member

Unfortunately this limitation seems to either clutter resource folders or cause us to have to take round about solutions. By RajawaliTemplate project are you referring to the one we have discussed creating? What about letting the packer take either a int resource id or an InputStream. That would not tie Rajawali to using assets or a template project (For my use case it would be an insane amount of work to bring Rajawali in through a template). Obviously this would leave the dev to deal with getting an InputStream, but it also opens up being able to stream from the web.

As for your side note, I personally would put both in rajawali.materials as the texture packer serves no other purpose. Id probably move some of the current util contents around as well but thats another issue.

@Davhed
Copy link
Member Author

Davhed commented Apr 30, 2013

That makes sense. I am already decoding InputStream from AssetManager, so that's probably the best divergence point. Then the developer can decide how they want to collect those data. Thanks for that input, and I think I'll keep this bit of code around because having an option to load from assets might be a nice to have feature in the end.

@jwoolston
Copy link
Member

@Davhed you could do all three even, I know I will use loading from assets and it would be nice to not have to write that a bunch of times. You could simply put in the docs that they need to have that folder in their app if they use it.

@Davhed
Copy link
Member Author

Davhed commented May 1, 2013

Since I am not an official contributor to Rajawali I have posted my progress to my own fork. It's very much WIP... currently reads in files and stores their data. Insight and feedback is always welcome.

https://github.com/Davhed/Rajawali/blob/texture_packing/src/rajawali/materials/TexturePacker.java

https://github.com/Davhed/Rajawali/blob/texture_packing/src/rajawali/materials/TextureAtlas.java

@MasDennis
Copy link
Member

I agree with the assets vs drawable issue. Assets would be a nice to have but loading from drawable is a must have.
Keeping it in rajawali.materials also has my preference.

I just glanced through your code and it looks great so far. A very clean API 👍

I have a question about Line 75 in TexturePacker. You throw an error when the size of the Bitmap is greater than the size of the texture atlas. I think it is better to resize the Bitmap and pack it anyway. The user can then determine whether the quality of the resized Bitmap is good enough or not. If it isn't they can increase the size of the texture atlas.

@ToxicBakery
Copy link
Member

I tried reading the thread but maybe I missed it, what is wrong with using res/drawable-nodpi? In other projects assets is used instead of resources for cross OS reasons. As this is not the case for Rajawali it seems at first glance res/drawable-nodpi is the best location for image resources as the files are guaranteed to be there.

@jwoolston
Copy link
Member

I agree with Dennis...if they use too large of a bitmap we should just use it. Maybe RajLog a warning but otherwise leave it to the developer's opinion.

@ToxicBakery I believe the consensus of the correct way to move forward was in line with using drawable-nodpi for the same reasons you mentioned.

@Davhed
Copy link
Member Author

Davhed commented May 1, 2013

This is awesome! Thank you all for your insights. Automatically reducing a large bitmap is smart. I'm learning so much from this, and i truly appreciate the guidance.

@jwoolston
Copy link
Member

@Davhed it goes both ways so thankyou 👍 You're questions are helping me refresh Android tidbits I learned in passing a while ago. One other consideration I just remembered which you may or may not be aware of and may or may not effect you. @MasDennis since you are working with new texture management it may impact you as well...

  • Since we have moved to 4.0+ support only, calling Bitmap.recycle() literally does nothing. Bitmaps are now managed on the heap so there is no native memory management to be performed. This only applies if you use the Android Bitmap class.

@jwoolston
Copy link
Member

Oh, and its probably fastest to use BitmapFactory.Options#inSampleSize http://developer.android.com/reference/android/graphics/BitmapFactory.Options.html#inSampleSize for the resizing.

@Davhed
Copy link
Member Author

Davhed commented May 2, 2013

For those interested, progress has been made. TexturePacker can now sort all incoming images in decending order by size, and is also able to handle multiple atlas pages in the case that the developer tries to pack more textures than can fit into a singe atlas of a given size. Additionally, while the code is still currently reading from assets (for my own short term convenience) the class is now able to accept an array of InputStream objects which should provide a pretty wide open interface for developers to handle the file collection as they please.

I'm still working on the packing algorithm which will eventually use a recursive function to navigate through a binary tree. I'm not great with recursion so this part may take me a bit longer... but I feel confident that it's coming together as I hoped.

Here's a screenshot showing a (poorly) packed atlas on a Plane. The vertical offset isn't working yet hence the orange square on the wrong line... but progress is progress!
screenshot_2013-05-01-18-38-14

@MasDennis
Copy link
Member

Thanks for the update :) Good to see you're making progress! Such a good feature to have for the framework.

So recycle() doesn't do much anymore. That is good to know. I'll have to cater for that in TextureManager.
The plan is to continue to work on that tonight. Although I'm not sure if I'll have the energy. I had too much Mongoose yesterday evening.

@AndrewJo
Copy link
Member

AndrewJo commented May 2, 2013

I had too much Mongoose yesterday evening.

So you basically had too much energy drink is what you're saying? 😆

@AndrewJo
Copy link
Member

AndrewJo commented May 7, 2013

Awesome!!! 👍

@MasDennis
Copy link
Member

Just look at that beautiful texture atlas 😂 Great job @Davhed!
I've also made good progress on TextureManager yesterday. Hopefully we'll be done around the same time so we can start integrating them.

@jwoolston
Copy link
Member

@Davhed I must say you picked an interesting range of beers there.....I highly approve of your first entry to the atlas though. Getting close!

@Davhed
Copy link
Member Author

Davhed commented May 8, 2013

The TextureAtlas now supports multiple pages and you can retrieve an atlas page by passing a resource file name. I would really like your feedback and recommendations for the remainder. Please have a look at the classes on my repo:

TexturePacker

TextureAtlas

This is how it is currently used:

        TextureAtlas mAtlas = new TexturePacker(mContext).packTexturesFromAssets("AwesomeAtlas", 1024, 1024, 0, false, "atlas");

        Plane mPlane = new Plane (2,2,1,1);
        mPlane.setMaterial(new SimpleMaterial());
        mPlane.addTexture(mTextureManager.addTexture(mAtlas.getTile("becks").getPage()));
        addChild(mPlane);

As you can see the "addTexture" portion is not ready... in fact that's where I'm a bit stuck. @MasDennis would like to see a single-step solution for passing in a texture, but I'm not sure how to approach it. What I am thinking is that the developer should be able to simply pass the name of the texture they would like to add, and behind the scenes a few things need to happen.

  1. Pass target texture name
  2. Retrieve the atlas page holding the target texture
  3. Scale the target objects UV based on the X,Y,Width,Height coordinates of that texture in the atlas

All the data needed is stored in the Tile objects that are compiled into the TextureAtlas object, I just don't really know how to accomplish this in one step. Therefore, I'm very open to suggestions or improvements to make this possible.

And just for fun here's another packed atlas showing a wide range of texture sizes:
screenshot_2013-05-07-17-46-39

@jwoolston
Copy link
Member

@Davhed Great work! Sorry it took so long to get a look at this. I have a few suggestions which may simplify all this but we should take into consideration the work @MasDennis is doing to revamp the TextureManager class. That said:

  • TexturePacker is only used for generating the TextureAtlas correct? After generation all further interaction goes to the TextureAtlas class? If so perhaps we should let TextureManager handle all of this:
mTextureManager.packTexturesFromAssets("AwesomeAtlas", 1024, 1024, 0, false, "atlas");
//Which then calls something like this internally
TextureAtlas mAtlas = new TexturePacker(mContext).packTexturesFromAssets("AwesomeAtlas", 1024, 1024, 0, false, "atlas");

This process would need to be worked into the task queue but we need to figure out how the texture stuff in general is handled there first.

  • What if TextureManager kept a HashMap like list of TextureAtlas objects. So you would have something like
"MyAtlas1" -> instance1_TextureAtlas
"MyAtlas2" -> instance2_TextureAtlas
etc

Then when you wanted to texture an object your call could look like this:

BaseObject3D.addTexture(mTextureManager.getTexture(String atlasName, String tileName));

This would essentially pass the Tile object to BaseObject3D through the addTexture() method. There is probably other data that needs to go with it depending on how textures will be referenced now but it is likely still going to be a TextureInfo object. If this is the case, the info should have a boolean for isPartOfAtlas or some such. Upon receiving this texture, if it is a part of an atlas the BaseObject3D instance should be responsible for scaling its own UV coordinates....its owns the data, let it handle it. It should also keep a copy of the original UV data and a flag for if it has been modified, so that if the texture is changed, either to a non atlas or a different atlas tile, it can be correctly restored without having to go through the inverse operation.

What do you think?

@Davhed
Copy link
Member Author

Davhed commented May 9, 2013

If so perhaps we should let TextureManager handle all of this:

Ah yes, clever. I appreciate the seamless approach here. Adding this implementation is the right way to go IMO.

What if TextureManager kept a HashMap like list of TextureAtlas objects. So you would have something like

Also makes sense, although I don't know how to accomplish this yet.

I really love the idea of getTexture(atlasName, tileName) being the method for retrieval. However, the current paradigm allows the user to apply a texture to a BaseObject3D OR a material. Would that be possible still? I don't have a great grasp of the interaction between the object and the material right now, but I'll dig in and do some learning today.

I like! Thanks @jwoolston! I would really like to hear any thoughts from @ToxicBakery @AndrewJo @jayschwa too. I know you guys are all busy, so please take your time.

@jwoolston
Copy link
Member

@Davhed here is the addTexture() method from BaseObject3D. It basically just calls through to its material.

public void addTexture(TextureInfo textureInfo) {
    if (mMaterial == null) {
        RajLog.e("[" + getClass().getName() + "] Material is null. Please add a material before adding a texture.");
        throw new RuntimeException("Material is null. Please add a material first.");
    }
    if (mLights.size() > 0 && textureInfo.getTextureType() != TextureType.SPHERE_MAP) {
        mMaterial.setUseColor(false);
    }
    mMaterial.addTexture(textureInfo);
}

@ToxicBakery
Copy link
Member

Actually I really don't like that you can add textures via BaseObject3D. This is frankly misleading and does not convey what is really going on in my opinion. Textures should really only be added to the materials. Additionally I would prefer setMaterial(AMaterial material, boolean copyTextures) be replaced with either a static or instance method that permits copying textures from one material to another.

Just my thoughts, these would likely be breaking changes and they might exist for reasons unknown to me so I'm not sure if it makes sense to actually remove those or if anyone even agrees with me.

@Davhed
Copy link
Member Author

Davhed commented May 9, 2013

OK so ideally this would happen in AMaterial

AMaterial.addTexture(mTextureManager.getTexture(String atlasName, String tileName));

What would be the link back to the BaseObject3D -> Geometry3D for updating the object's UVs?

@jwoolston
Copy link
Member

Actually I really don't like that you can add textures via BaseObject3D.

I share this sentiment actually, so if people agree with removing it, lets do it. We have broken just about everything else, if it improves things, now is the time to do it. Hypothetically speaking, if we removed the call on BaseObject3D, how would we go about modifying the texture coordinates?

Additionally I would prefer setMaterial(AMaterial material, boolean copyTextures) be replaced

What does this method do? I haven't been able to figure it out.

@ToxicBakery
Copy link
Member

the copyTextures flag just permits copying any textures on the currently set material to the material that is about to be set. Its just a convenience method but I think it belongs on AMaterial and not BaseObject3D.

@jwoolston
Copy link
Member

Ah, ok that makes more sense.

@ToxicBakery
Copy link
Member

Can you pull this change in and apply it liberally. Make sure you let it soak in for 30 minutes before going outside.

#684

@Davhed
Copy link
Member Author

Davhed commented May 10, 2013

clutzblock SPF 50 has been applied, I will add further checks in the TexturePacker

@Davhed
Copy link
Member Author

Davhed commented May 10, 2013

Seems like a "Texas 2-step" solution is going to be the best bet based on this discussion #600

  • Get the correct atlas page
  • Update UVs on the mesh

@Davhed
Copy link
Member Author

Davhed commented May 11, 2013

@MasDennis I'm sorry... I couldn't help myself. I added addTexture(TextureAtlas atlas, String tileName) to TextureManager because I wanted/needed to see it work.

I have also modified BaseObject3D to include setAtlasTile(TextureAtlas atlas, String tileName) for coordinate-UV conversion.

So, it is now working in a pretty simple 2-step process:

        TextureAtlas mAtlas = new TexturePacker(mContext).packTexturesFromAssets(1024, 1024, 0, false, "atlas");

        SimpleMaterial mSimpMat = new SimpleMaterial();
        mSimpMat.addTexture(mTextureManager.addTexture(mAtlas, "newcastle"));

        Plane mPlane = new Plane (2,2,1,1);
        mPlane.setMaterial(mSimpMat);
        mPlane.setAtlasTile(mAtlas, "newcastle");       
        addChild(mPlane);

I still need to check my UV conversion algorithm with more complex models/atlases, but I hope I can get a little assistance with the battle testing.

This is the updated branch:
https://github.com/Davhed/Rajawali/tree/texture_packing

@Davhed
Copy link
Member Author

Davhed commented May 11, 2013

A method for compressing the atlas bitmaps should be included too. I almost forgot about that. TextureAtlas already has a boolean flag for this purpose, so I suppose it just needs to be evaluated in TextureManager and have the appropriate handler called. I will wait, for reals this time, until TextureManager is ready for action

@AndrewJo
Copy link
Member

I can add in the compression support when this is working first for regular uncompressed textures.

@Davhed
Copy link
Member Author

Davhed commented May 11, 2013

Cool, it is to that point, but should get some peer review

@Davhed
Copy link
Member Author

Davhed commented May 11, 2013

Actually it apparently needed some "beer" review... I just removed some hard coded references to Newcastle... it made me LOL

@MasDennis
Copy link
Member

Haven't forgotten about this @Davhed. Will pick this up after the skeletal animation task.

Or you could submit a pull request with the way you see it working. Then we can all review it and merge it if its all good 🍻.

@ToxicBakery
Copy link
Member

As I was waiting for the texture atlas code to come online I jumped onto the AWD parsing. If anyone wants to knock out the font to texture support I wont take offense =)

I am guessing @Davhed would know best as he already figured out the packing which was the next step to figure out in what I was working on.

@Davhed
Copy link
Member Author

Davhed commented May 22, 2013

Closing this now since the pull request is posted #782

@Davhed Davhed closed this as completed May 22, 2013
@Davhed Davhed removed their assignment Mar 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants