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

Handle empty vertex buffers in GUIFontTTFGL #15358

Merged
merged 1 commit into from Feb 5, 2019

Conversation

@pkerling
Copy link
Member

commented Jan 29, 2019

CGUIFontTTF::DrawTextInternal can end up with zero-element vertex
buffers in corner cases such as when all text to render is completely
clipped away. This should also be cached as such, so it is unfeasible to
forbid empty vertex buffers completely. CGUIFontTTFDX already handles
this case, but CGUIFontTTFGL did not.
A previous crash fix added an assertion on the vertex list not being
empty, but this is not needed anymore since that case is correctly
handled now.

CGUIFontTTF::DrawTextInternal can end up with zero-element vertex
buffers in corner cases such as when all text to render is completely
clipped away. This should also be cached as such, so it is unfeasible to
forbid empty vertex buffers completely. CGUIFontTTFDX already handles
this case, but CGUIFontTTFGL did not.
A previous crash fix added an assertion on the vertex list not being
empty, but this is not needed anymore since that case is correctly
handled now.
@pkerling

This comment has been minimized.

Copy link
Member Author

commented Jan 29, 2019

Context: #15104 (comment)

@pkerling pkerling self-assigned this Jan 29, 2019
@pkerling pkerling added the v18 Leia label Jan 29, 2019
@pkerling pkerling added this to the Leia 18.1-rc1 milestone Jan 29, 2019
@pkerling pkerling requested a review from FernetMenta Jan 29, 2019
@pkerling

This comment has been minimized.

Copy link
Member Author

commented Jan 29, 2019

@FernetMenta Can you confirm it fixes the crash?

@enen92

This comment has been minimized.

Copy link
Member

commented Jan 29, 2019

@pkerling maybe also fixes #15290 ? @basrieter

@pkerling

This comment has been minimized.

Copy link
Member Author

commented Jan 29, 2019

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Jan 30, 2019

@pkerling don't wait for me to confirm this. my coding/testing time is limited. btw: I just dropped the assert here and it seemed to be fine.

@pkerling

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2019

@FernetMenta This will go into 18.1 at the earliest, so I'm not in a hurry. Just report back when you have the time and I'll merge it for 18.1 rc1 if I don't hear from you.

Removing the assert might be fine since I changed &vertices[0] to vertices.data() in the last fix (which should also prevent the stdc++ assertion from triggering), but I wouldn't rely on it - don't know how GL libs handle empty array buffers for example. This here is the correct fix.

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Feb 4, 2019

Can we merge this? I'd like to start getting an RC out and go for 18.1 as we already fixed some stuff

@pkerling

This comment has been minimized.

Copy link
Member Author

commented Feb 4, 2019

@MartijnKaijser go ahead

@MartijnKaijser MartijnKaijser merged commit 661d015 into xbmc:master Feb 5, 2019
1 check passed
1 check passed
default You're awesome. Have a cookie
Details
@pkerling pkerling deleted the pkerling:guifontgl-empty-vertices branch Feb 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.