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

FIX: Correction of height and width in extracted canvas #5842

Merged
merged 5 commits into from Sep 21, 2019
Merged

FIX: Correction of height and width in extracted canvas #5842

merged 5 commits into from Sep 21, 2019

Conversation

martinboksa
Copy link
Contributor

@martinboksa martinboksa commented Jun 26, 2019

Description of change

Correction of width and height in extracted canvas.

Some devices returns width and height with decimal places ( 'cause resolution is 2.758, etc.. ). So later ( in computation ) the size of data will not equal.

Example:

const width = frame.width * resolution // 1182.5;
const height = frame.height * resolution // 1012;
const webglPixels = new Uint8Array(BYTES_PER_PIXEL * width * height); 
const canvasData = canvasBuffer.context.getImageData(0, 0, width, height);

// webglPixels.length = 4 786 760
// canvasData.data.length = 4 784 736
// webglPixels.length !== canvasData.data.length

canvasData.data.set(webglPixels);

It caused the error

RangeError: Source is too large.

Reproduction

Chrome: android devices like: xiaomi mi mix 3
My colleague has also succeeded in the chrome mobile emulator

@themoonrat
Copy link
Member

Are you able to come up with a pixiplayground or jsfiddle with this error occuring? I've done a pr for v5 fixing a similar thing, so a shared example would be great

#5836

@martinboksa
Copy link
Contributor Author

martinboksa commented Jun 26, 2019

@ivanpopelyshev ivanpopelyshev added the 🛸 X-Files Mysterious, unexplainable or elusive behavior that’s difficult to track down or reproduce. label Jun 26, 2019
@ivanpopelyshev
Copy link
Collaborator

ivanpopelyshev commented Jun 26, 2019

Welcome to the X-Files. You can see all those resolution rounding PR's and issues in the list by clicking on a aqua-colored label.

I need more info about it, how exactly did you get 1182.5 there? It seems that there're more places that are affecting that value, not only this floor

@ivanpopelyshev
Copy link
Collaborator

ivanpopelyshev commented Jun 26, 2019

Usually I do ceil(x-eps) in such places, where eps=1e-4.

@martinboksa
Copy link
Contributor Author

martinboksa commented Jun 26, 2019

The error appeared on Xiaomi mi mix 3. I was trying to get inner container screenshot with these specs.

Specs:

// frame.width = 430
// frame.height = 368
// resolution = 2.75

const width = frame.width * resolution // 430 * 2.75 = 1182.5;
const height = frame.height * resolution // 368 * 2.75 = 1012;
const webglPixels = new Uint8Array(BYTES_PER_PIXEL * width * height);  // 4 * 1182.5 * 1012 = 4 786 760
const canvasData = canvasBuffer.context.getImageData(0, 0, width, height); // 4 * 1182 * 1012 = 4 784 736

// webglPixels.length !== canvasData.data.length

canvasData.data.set(webglPixels);

@ivanpopelyshev
Copy link
Collaborator

Awesome! Can you also give me renderer.gl.drawingBufferWidth and height please?

@ivanpopelyshev
Copy link
Collaborator

ivanpopelyshev commented Jun 26, 2019

Please add + (1e-4) inside floor operation and I'll approve it. I know cases when certain zoom values affect multiplication/division precision.

@bigtimebuddy bigtimebuddy modified the milestones: v5.1.0, v4.8.9 Jun 27, 2019
@martinboksa
Copy link
Contributor Author

martinboksa commented Jul 2, 2019

I forgot which sprites were used.
So another example:

frame: width,height = [188, 250]
width, height = [517, 687.5]
resolution = 2.75
canvasData.data.length = 1420716
webglPixels.length = 1421750
renderer.gl.drawingBufferWidth = 1080

@ivanpopelyshev
Copy link
Collaborator

Oh, you've used extract on sprites, not whole screen. Why do you need them? If its just a single sprite - there are better ways to do that.

@martinboksa
Copy link
Contributor Author

I extracted container. Because I've got a products and i need to take container from one product and then move it into another product.
Anyway what's better way to do that with single sprite?

@ivanpopelyshev
Copy link
Collaborator

ivanpopelyshev commented Jul 3, 2019

Is it in the same renderer, webgl context, canvas, or in different?

@martinboksa
Copy link
Contributor Author

Is it in the same renderer, webgl context, canvas, or in different?

different

themoonrat added a commit that referenced this pull request Jul 5, 2019
@pixijs pixijs deleted a comment from Riggin-GOD91 Aug 14, 2019
@pixijs pixijs deleted a comment from Riggin-GOD91 Aug 14, 2019
@bigtimebuddy bigtimebuddy added the 💾 v4.x (Legacy) Legacy version 4 support label Aug 17, 2019
@bigtimebuddy bigtimebuddy merged commit 3bf4e47 into pixijs:v4.x Sep 21, 2019
@bigtimebuddy
Copy link
Member

Thank you @martinboksa, sorry for the delay. Hoping to get a v4 release out soon.

@martinboksa
Copy link
Contributor Author

@bigtimebuddy no problem, I finally updated to 5.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💾 v4.x (Legacy) Legacy version 4 support 🛸 X-Files Mysterious, unexplainable or elusive behavior that’s difficult to track down or reproduce.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants