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

Add chances to export textures in internal buffer #208

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

JochnGst
Copy link

I add to the GLTFSceneExporter to major changes.
First I add a Methode to append bytes to the buffer, if it is not a multiply to 4.

the secound change is to put textures in the internal buffer if export as GLB is selected.

Copy link
Contributor

@StephenHodgson StephenHodgson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's revert some of the extra line spaces and formatting changes to make the review a bit easier.

Copy link

@blgrossMS blgrossMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and fixes the tabs and spaces for you. There are a couple of notes that I had

@@ -1236,6 +1316,21 @@ private AccessorId ExportAccessor(int[] arr, bool isIndices = false)
return id;
}

private long AppendToBufferMultiplyOf4(long byteOffset, long byteLength)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be simplified to: public static uint CalculateAlignment(uint currentSize, uint byteAlignment)
{
return (currentSize + byteAlignment - 1) / byteAlignment * byteAlignment;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the length calculation

@@ -1221,9 +1298,12 @@ private AccessorId ExportAccessor(int[] arr, bool isIndices = false)

accessor.Min = new List<double> { min };
accessor.Max = new List<double> { max };
//byteOffset = AppendToBufferSizeTo4(accessor, byteOffset);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove?


var byteOffset = _bufferWriter.BaseStream.Position;

{//

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove?


{//
var destRenderTexture = RenderTexture.GetTemporary(texture.width, texture.height, 24, RenderTextureFormat.ARGB32, RenderTextureReadWrite.sRGB);
GL.sRGBWrite = true;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all texture are sRGB

@CLAassistant
Copy link

CLAassistant commented Jul 2, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Jochen Gast seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants