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

Use appropriate UNPACK_ALIGNMENT for data textures #13013

Merged
merged 2 commits into from
Nov 20, 2021

Conversation

mike-000
Copy link
Contributor

Fixes #12995

Most of the time (RGBA data and/or standard tile sizes) the default UNPACK_ALIGNMENT value of 4 has no problems. However a combination of RBG or other data and non-standard tile sizes may mean this does not work. The calculations assumed an UNPACK_ALIGNMENT of 1 (not a valid assumption) but continued to use the default value, It is possible to avoid assumptions and calculate an appropriate value from the data size and byteLength and set that when uploading data textures, then restore the default value. It should not be necessary for the application to guess a value and patch it in the prerender event.

@github-actions
Copy link

📦 Preview the examples and docs from this branch here: https://deploy-preview-13013--ol-site.netlify.app/.

@mike-000
Copy link
Contributor Author

I used a modified version of the Data Tiles examples https://codesandbox.io/s/data-tiles-forked-yx559?file=/main.js to check the calculations work - it could be used as the basis for rendering tests if needed.

@@ -84,6 +93,7 @@ function uploadDataTexture(helper, texture, data, size, bandCount) {
textureType,
data
);
gl.pixelStorei(gl.UNPACK_ALIGNMENT, 4);
Copy link
Member

Choose a reason for hiding this comment

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

We can restore this to the previous gl.getParameter(gl.UNPACK_ALIGNMENT) value. I've changed this in the latest commit.

data,
size,
bandCount,
unpackAlignment
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding another argument, this function can determine the optimal unpack alignment based on data.byteLength / size[1]. I've changed this in the latest commit.

@tschaub
Copy link
Member

tschaub commented Nov 20, 2021

Thanks for identifying this issue and putting together a fix, @mike-000.

I think it is worth describing two separate issues that your change addresses.

The first issue is with texture data where the "byte width" (or number of bytes per row) is not divisible by 4. This is the issue described in #12995 (e.g. 3-band, 8-bit data and a tile width of 25). This can be addressed by setting the appropriate gl.UNPACK_ALIGNMENT.

The second change you've made here is to allow "loosely packed" data tiles (where the band count x tile width x tile height is not the same as the length of the data array). I'm not sure when we might encounter these. But I think it makes sense to respect the provided tile width (and height) in this case.

I've added rendering tests for these two separate cases and reworked the alignment handling a bit.

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.

Black tiles with DataTileSource and WebGLTileLayer
2 participants