Repository navigation
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces clamping for texture dimensions in the TextureGLES constructor to ensure a minimum size of 1x1. However, the implementation uses const_cast on a constant reference and performs unsafe type-punning between ISize and Point, which results in undefined behavior. The feedback suggests refactoring the clamping logic into the member initializer list to ensure the base class is initialized with valid, immutable data.
702c6ca to
6fdc911
Compare
|
I have read the CLA Document and I hereby sign the CLA |
75a1c24 to
df049ad
Compare
|
I have read the CLA Document and I hereby sign the CLA |
gaaclarke
left a comment
There was a problem hiding this comment.
@zendy199x can you please add a unit test for this in texture_gles_unittests.cc
|
When constructing a texture with an empty size,
Created a PR that does this: #184377 |
c0dd632 to
ca7ad5c
Compare
|
I have read the CLA Document and I hereby sign the CLA |
…utter#179572) The issue occurs when VideoPlayer is wrapped in a SizedBox with dimensions < 1, causing texture descriptors with zero or near-zero dimensions to be created. This breaks Impeller rendering with "Invalid texture descriptor" error. The fix is to clamp texture dimensions to at least 1 pixel before creating the texture. Signed-off-by: Zendy <50132805+zendy199x@users.noreply.github.com>
6099521 to
ded683a
Compare
|
This issue has been fixed by 9eb3812 |
Fixes #179572
This PR fixes an issue where VideoPlayer wrapped in a SizedBox with dimensions less than 1 would cause zero or near-zero texture dimensions, breaking Impeller rendering with an "Invalid texture descriptor" error. The fix clamps texture dimensions to at least 1 pixel in the TextureGLES constructor to prevent invalid descriptors from being created.
Fixes issue where zero-sized textures could be created when VideoPlayer is wrapped in a SizedBox with dimensions < 1.
This change affects the texture creation process in the GLES backend and ensures that all textures have valid dimensions before being used in rendering.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.