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

Omit leading zeros when serializing float Attribute (and other Attributes consisting of floats) into TXML #768

Open
Stinkfist0 opened this issue Jul 30, 2014 · 11 comments

Comments

@Stinkfist0
Copy link
Contributor

Currently f.ex. a default constructed Transform attribute looks like this in TXML:

<attribute value="0.000000,0.000000,0.000000,0.000000,0.000000,0.000000,1.000000,1.000000,1.000000" type="Transform" id="transform" name="Transform"/>

By omitting unnecessary leading zeros in floats we would save a lot of bytes:

<attribute value="0,0,0,0,0,0,1,1,1" type="Transform" id="transform" name="Transform"/>
@jonnenauha
Copy link
Member

Yes please :)

@jonnenauha
Copy link
Member

Should be fairly trivial too, util func to remove all trailing 0s.

Here https://github.com/realXtend/tundra/blob/tundra2/src/Core/TundraCore/Scene/IAttribute.cpp#L158-163

Then float2/3/4/Quat in the MGL classes https://github.com/realXtend/tundra/blob/tundra2/src/Core/TundraCore/Scene/IAttribute.cpp#L170

@jonnenauha
Copy link
Member

Btw is it ok to modify the MGL classes in Tundra? I mean there is some kind of manual merge/diffing step each time we want a new version of MGL in? Are these mods going to make that harder for @cadaver or whoever does that?

@jonnenauha
Copy link
Member

Actually maybe its better to just do the to string logic in IAttribute (copying the format from matlib) and do all that stuff via QString::number() etc. So we dont have to touch the math classes.

@Stinkfist0
Copy link
Contributor Author

Yeah it's probably better to do in IAttribute in order to keep the Tundra-specific mods of MGL at minimum.

@Stinkfist0 Stinkfist0 changed the title Omit leading zeros when serializing float Attribute (or another Attribute consisting of floats) into TXML Omit leading zeros when serializing float Attribute (and other Attributes consisting of floats) into TXML Jul 30, 2014
@cadaver
Copy link
Member

cadaver commented Jul 30, 2014

Yes, the more we mod MGL, the more troublesome it is to merge a new version. That is a manual process as Tundra's MGL is just files in the Tundra repo.

Therefore this is indeed preferable to do in eg. IAttribute.

@Stinkfist0
Copy link
Contributor Author

However, I see no harm in pinging clb regarding this matter; maybe he'd prefer to have this in MGL.

@jonnenauha
Copy link
Member

I'm sure the only thing where Jukka uses strings is debug prints if even there, but still reading that stuff is much nicer without extra zeros.

@juj Any input? :)

@juj
Copy link
Contributor

juj commented Jul 30, 2014

There were functions X::SerializeToString() added to MathGeoLib some time ago, see here juj/MathGeoLib@40d3981

Those functions have the two features:

  • X::FromString(obj.SerializeToString()) == obj
  • The representation of obj.SerializeToString() should be minimal but readable without extra characters.

Try that form, in particular the %.9g should omit trailing zeros, as opposed to the older %f that was used.

@Stinkfist0
Copy link
Contributor Author

Ok, nice, I think we can pretty safely modify our MGL copy to use the 'g' modifier.

@jonnenauha
Copy link
Member

Sure would be nice to sync whole MGL into Tundra again, seems out last update has been 1 year ago :( SerializeToCodeString also looks it might be useful in Tundra code.

What was the main reason of keeping a copy in Tundra source tree instead of using it as a pre-built dependency?

Stinkfist0 added a commit to Adminotech/tundra that referenced this issue Jul 31, 2014
…lor, Quat and Transform Attributes into a string. Closes realXtend#768.
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

4 participants