KhronosGroup / OpenCOLLADA

652 stars 252 forks source link

Implement embedded/data image support #415

Open m-7761 opened 8 years ago

m-7761 commented 8 years ago

I'd like to insist that the team get around to implementing the Collada 1.5 HEX element for embedded images. I think 1.4.1 has DATA or something like this instead. From COLLADAFWImage.h:

    /** Contains a sequence of hexadecimal encoded binary octets composing the embedded
    image data.*/
    // Disable until we really need it to avoid problems with copy/assignemnet operators
    //CharacterArray mData;

How do we define "really need it" here? Can one person's request be enough? Here (https://developer.blender.org/T47999) is a link that includes a link to a DAE file with an embedded image, if you can use one for testing purposes.

I can see about working on this myself if necessary. I am in the processes of reviving/upending the original Collada-DOM project, and I need a peer application for my purposes, and Blender is the only appropriate candidate that I know of. It is using OpenCollada, so it cannot be fixed in Blender.

Blender opens the file in the link. It says the file is invalid around the element. I think OpenCollada is probably vouching for its supposed validity, but I do not know for sure. I am working on getting debug symbols working with the Blender right now. I noticed though that this is commented out in the header.

Thank you. Also hello Remi. I recognize your name. I've been chatting with Mark Barnes somewhat the last few weeks. I hope you two are still tight.

m-7761 commented 8 years ago

If it's any help. The HEX element seems to be parsed as a Uint8. I think it fails on the hexadecimal, as it's not a decimal. Collada-DOM has a similar limitation, due to missing a hexBinary datatype. Just a guess. Here is some illustrative code, but it's procedurally generated:

bool ColladaParserAutoGen15Private::_datalibrary_imagesimageinit_fromhex( const ParserChar* text, size_t textLength ) {

ifdef GENERATEDSAXPARSER_VALIDATION

if ( mValidate )
{

return characterData2Uint8Data(text, textLength, &ColladaParserAutoGen15::datalibrary_imagesimageinit_fromhex, 0, 0, 0); } else { return characterData2Uint8Data(text, textLength, &ColladaParserAutoGen15::datalibrary_imagesimageinit_fromhex); }

else

{

return characterData2Uint8Data(text, textLength, &ColladaParserAutoGen15::datalibrary_imagesimageinit_fromhex); } // validation

endif

}

RemiArnaud commented 8 years ago

@mick-p1982 I don't know of anyone doing 1.5 development at this time, but we'd gladly take your contribution if you want to open a PR

m-7761 commented 8 years ago

@Remi I cannot get to invested in OpenCollada. I do not mind doing spot fixes. It seems incumbent on someone to do 1.5. Were it up to me, being newer, the policy would be to make 1.5 work fully, and only then consider 1.4.1. I realize 1.4.1 is more commonly used by files, and it's a standalone format, however there must be a reason 1.5 exists, and if we only look backward, well you know how this sentence ends :)

(This cannot be too difficult a job. To fix it in Collada-DOM only takes maybe 15 minutes, and I imagine it's the same here, more or less. There's something to be said for working as advertised. I will keep an eye on this.)

RemiArnaud commented 8 years ago

1.5 is used a lot in robotic and car industry (for robots as well)

See http://openrave.org/docs/0.8.2/collada_robot_extensions/

m-7761 commented 8 years ago

Just for the record, to fix this the generator must be changed, as it's expressly saying characterData2Uint8Data and not hexBinary or something like that.

BTW: off-topic, but I discussed this with Mark, and I don't know if he was speaking hypothetically, but he agreed with me that should've been hexBinary type, and not list_of_hexBinary. He thought it should be changed in the 1.5 stream, but didn't say if that meant 1.5.1 or if the XSD file on Khronos could simply be replaced overnight.

It's something that bugs me, but I figure it's stuck that way, and so Collada-DOM will have to have a union or something to support a separate channel of bit-ranges to indicate where the spaces are to be.

Logically HEX represents an IMAGE and not an "IMAGES", so it's borderline nonsensical for it to be a list. It seems like it could be categorized as a typo, but Mark mentioned that you'd made it a List type, thinking someone might use it, for mipmaps or atlases, or arbitrary purposes, but that seems like a bad idea also, and those things can be and are normally done in the image header.

I only mention it, because if literally no one is using this feature, maybe this could all get corrected at once (I am also not playing devil's advocate. I have model files with images embedded in them, and I see no reason to automatically externalize the images. But neither is this my sole interest in Collada. I am working full-time in 2016 to completely vitalize Collada, at least in my neck of the woods)

PS: here is a brochure about my interest in Collada (https://www.patreon.com/swordofmoonlight) --Actually the first/only patron is Mark. Maybe you'd like to join him? It would be pretty cool to have both of you guys to begin with, but maybe this isn't up your alley. It's not a new project, but it's been slow to gain traction with the public at large. There is also a revolutionary graphical technique in the middle of it, which is interesting in its own right. I've described it to Mark in the Comments there.