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

Added support for S3TC textures in ImageTextureAtlas. #538

Merged
merged 4 commits into from Sep 4, 2015

Conversation

Projects
None yet
3 participants
@girier-tsha
Contributor

girier-tsha commented Aug 20, 2015

ImageTextureAtlas node has been extended to handle compressed (S3TC) textures by simply setting the "compressed" boolean field to true.

In case the necessary extension (WEBGL_compressed_texture_s3tc) is not supported by the browser, the "fallbackUrl" field can be set to the URL of an equivalent uncompressed image (See src/Texture.js, line 267 and following).

The actual logic for reading an s3tc fileis contained in src/util/Utils.js (x3dom.Utils.createCompressedTexture2D).
The src/Cache.js file is only adding caching feature to this behavior (x3dom.Cache.prototype.getCompressedTexture2D).

@mlimper

This comment has been minimized.

Contributor

mlimper commented Aug 20, 2015

Thanks for the contribution!

A question about the "fallbackUrl" (-> "fallbackURL" ?) field:

Actually, we should already have fallback URLs, that's why url should be a multi-field (MFString). At least that's the intention in the X3D standard. The idea is that the X3D browser can try the first texture URL, and then proceed with following ones if that didn't work.

I think we can have the same here if we suppose for a moment that compressed textures could be identified by their URL. That's another question, of course. In case we are not able to identify them via their URL in general, the "compressed" field could be an array of boolean values, where each field corresponds to an entry in the "URL" field.

To sum up, I think an explicit fallbackURL field does not make sense, since the existing url field should already be used to provide fallback urls - because of that, it would be confusing to have another "fallbackURL" field that is only used for the case of compressed textures.

@andreasplesch

This comment has been minimized.

Contributor

andreasplesch commented Aug 20, 2015

Yes, the url field is supposed to provide alterntive urls if one is not interpretable by the browser:

http://www.web3d.org/documents/specifications/19775-1/V3.3/Part01/components/networking.html#X3DUrlObject

Would it be possible to automatically recognize S3TC as a supported image format, such as .jpg, in order to avoid having to introduce the "compressed" field ? The logic already checks for the DDS_MAGIC number, right ?
(One way to do that may be to move the logic into the image.onerror() function in utils.createTexture2d() . Hopefully, browsers would fire an error when it tries to load a s3tc file as an image?)

Then ImageTexture would be able to benefit from this great contribution without change as well.

However, I am not sure if x3dom currently actually supports using alternative URLs from the url field for any node (other than ExternalGeometry) in case of failure with one URL. It looks like it may not for ImageTexture. See issue #435. I believe this would need to be fixed first.

So at this point it may be necessary to work around with a fallbackurl field.

@mlimper

This comment has been minimized.

Contributor

mlimper commented Aug 21, 2015

So at this point it may be necessary to work around with a fallbackurl field.

I agree that it would be nice if #435 could be resolved first, but I would vote against working around using a "temporary" field. The reason is simply that I'm not aware of any case where a temporary field really got removed later on (if it's in the dev version, people will directly use it and complain later on if something changes).

Maybe this pull request could provide a workaround that only implements a fallback solution (using the regular URL field) for the case of compressed textures, and the more general implementation of #435 could build on this initial solution.

@andreasplesch

This comment has been minimized.

Contributor

andreasplesch commented Aug 21, 2015

I completely agree, and this is also why it would be best not to introduce a 'compressed' field. The logic flow could be:
use browser to load url as regular image (jpg,gif) → if error, try as s3tc texture → if error, try next url from regular URL field → if no more url → black?
But talk is cheap and some work would be required ...

issue #435 can be divided into a general part and a node specific part. The general part would address the situation where an url is not reachable, or returns no content. The node specific part needs to address if the content is not "interpretable", eg. is not a valid x3d file (for inline) or valid image (for imagetexture) but something else (html).

@mlimper

This comment has been minimized.

Contributor

mlimper commented Aug 21, 2015

Okay, sounds great, so let's figure out a proposal what @girier-tsha could do right now. I would propose this:

  • Use URL multi-field (MFString) instead of using an explicit fallbackURL field
  • Perform checks to determine if the texture behind a URL is compressed (and remove "compressed" field)

It is probably not easy to find a good solution for the second point, but what @andreasplesch proposed seems like a good starting point.

Thanks again for providing this contribution, this will be very useful for a lot of applications!

@girier-tsha

This comment has been minimized.

Contributor

girier-tsha commented Aug 25, 2015

Thanks for your comments and suggestions.

I was very reluctant to add the compressed and fallbackUrl fields and totally agree that it would be better to do without them.

I considered using the URL list as is specified in the X3D specification instead of fallbackUrl, but as said in issue #435, this behavior is not implemented in X3DOM. Solving #435 would effectively remove the need for the fallbackUrl field.

As for the compressed boolean field, I added it because I saw no other way to make sure the image is a compressed texture without downloading it twice. It is my understanding that the solution proposed above by @andreasplesch will download the image twice, once through setting the src field of an Image instance, and then by getting the binary data directly through an XMLHttpRequest. To my knowledge, there is no way to download the binary once, check its content and only then decide wether to generate a Image instance if it is a JPEG/GIF or to handle it as compressed texture. But maybe I am wrong about this. Please tell me if such a method exists. Anyway, as long as this inefficiency is not an issue to you, i see no problem in implementing it this way.

I also considered checking the URL extension to decide if the image is a JPEG or a X3TC file, but it seems far too unreliable to me.

Finally, I think the two issues can be solved separately.

As for #435, wouldn't a simple for loop at the end of the updateTexture function of src\Texture.js (around this.texture = this.cache.getTexture2D(...) solve the problem ?

As for the removal of the compressed field, I will first try and implement the proposal form @andreasplesch. Inside the src\util\Utils.js file, createTexture2D method, image.onerror handler:

try download as JPEG
  if error
    if error is 404 (or similar) || X3TC not supported -> try next URL
    else 
      try download as X3TC
      if error -> try next URL

Does this approach seems practical to you ?

@andreasplesch

This comment has been minimized.

Contributor

andreasplesch commented Aug 25, 2015

Thank you much for taking the time to go beyond what your needs for the node are.

Presumably, the browser will cache the image after downloading it the first time ? So hopefully the second attempt would not involve actual remote access and therefore still have satisfactory performance for your purposes. (Due to CORS remote access may be rare anyways).

A method may be to convert to dataurl after the XMLHttpRequest but see
http://stackoverflow.com/questions/8778863/downloading-an-image-using-xmlhttprequest-in-a-userscript
Ok, this is superseded by https://developer.mozilla.org/en-US/docs/Web/API/Blob
The documentation specifically mentions how to use this as an image.src. However, the feature is only supported by the latest generation of browsers.

Not sure where the best place is to iterate through the urls of the MFString url field. @mlimper may have more insight.

Perhaps a general function can be made which returns a reordered MFString which has the first accessible URL in the first (index [0]) position, and all inaccessible URLs at the end ? Could src/util/DownloadManager.js be used in some way to find the first accessible url ? Such a function could then be reused for other nodes.

@girier-tsha

This comment has been minimized.

Contributor

girier-tsha commented Aug 27, 2015

I followed the suggestion from @andreasplesch and made it so that, if loading a texture as a jpeg/gif/png fails, X3DOM tries to load it as a compressed texture if the corresponding WebGL extension exists on the client platform.

The fields compressed and fallbackUrl where removed from ImageTextureAtlas.

As an automagical side effect, compressed textures are now available for ImageTexture as well (^o^)/ !

I did not address #435, as it is another issue of its own and requires more change to the X3DOM source code than I am confident in doing without breaking anything else. In particular, some kind of feedback would be necessary so that the x3dom.Texture.updateTexture method can know if its calling cache.getTexture2D was successful or not. More precisely, we should have a kind of callback in the context of x3dom.Texture.updateTexture (with knowledge of the url list) which gets called if the texture creation fails and tries the next url.

Presumably, if someone was to fix #435, the functionality of fallbackUrl could be achieved by setting the url field to a value like compressed_image_url.xtc regular_image_url.png. On a client who does not support the S3TC extension, X3DOM would then try to load compressed_image_url.xtc, fail, and finally load and display regular_image_url.png.

@mlimper

This comment has been minimized.

Contributor

mlimper commented Aug 31, 2015

The fields compressed and fallbackUrl where removed from ImageTextureAtlas.

As an automagical side effect, compressed textures are now available for ImageTexture as well (^o^)/ !

Very nice :-D

Before we merge this one, I have only one / two more things:
To test this, as well as to showcase it, it would be great to have a test case. You can add it to the automated test suite.
This test case can then also be shown as a tech demo on examples.x3dom.org and shown as part of a blog post on x3dom.org (I'll take care of both) .

@girier-tsha

This comment has been minimized.

Contributor

girier-tsha commented Sep 4, 2015

I added the test suite for the compressed texture.

mlimper pushed a commit that referenced this pull request Sep 4, 2015

Max Limper
Merge pull request #538 from girier-tsha/master
Added support for S3TC textures

@mlimper mlimper merged commit 7fd8902 into x3dom:master Sep 4, 2015

@mlimper

This comment has been minimized.

Contributor

mlimper commented Sep 4, 2015

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment