Don't throw in GetProposedDimensions
#10260
Conversation
There was a problem hiding this comment.
Can we also just make the font fallback loader non-fatal? There are a bunch of THROW_IF_FAILEDs in DxFontRenderData::NearbyCollection, which contributes to this bug blowing up.
The caller, DxFontReenderData::_FindFontFace should treat failures coming out of NearbyCollection as non-fatal -- you can probably wrap the entire "if the system collection missed" conditional in a try/CATCH_LOG.
|
@DHowett I'm pretty those are all in try/catch's. It's just they are all bubbling up to an HRESULT, getting here, and this particular method is throwing it. |
|
Failing out the entire font load is the quickest way to get to a DX renderer that just doesn't have a font, and fails every time it tries to get one (which will probably result in a black screen?). It wouldn't matter if we caught the error at the highest level, the renderer would remain inoperable. Wrapping a localized try/catch around the nearby font collection lets fallback proceed as normal... which is really what we want, isn't it? You can probably mimic this failure with a EDIT: and setting an invalid font ofc |
|
@miniksa may need to re-validate this w/ the new changes to Nearby |
|
Hello @miniksa! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 2 hours 17 minutes. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
|
@msftbot wait 6 minutes before merging |
|
Hello @miniksa! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
I cannot for the life of me repro the original bug. I've got fonts with bad permissions SxS, I've tried installing a font twice, I've tried stopping the font cache service. No idea how to manually repro the original bug. BUT theoretically, this function should never throw. So lets just switch this to a `LOG_IF_FAILED`, and hope that this goes away? * [x] Fixes #10211? * [x] built & ran manually. Unclear if this can get cherry-picked trivially to 1.8. Code's pretty trivial though so if we need another PR for that, it can be arranged. (cherry picked from commit 89ca2ae)
I cannot for the life of me repro the original bug. I've got fonts with bad permissions SxS, I've tried installing a font twice, I've tried stopping the font cache service. No idea how to manually repro the original bug. BUT theoretically, this function should never throw. So lets just switch this to a `LOG_IF_FAILED`, and hope that this goes away? * [x] Fixes #10211? * [x] built & ran manually. Unclear if this can get cherry-picked trivially to 1.8. Code's pretty trivial though so if we need another PR for that, it can be arranged. (cherry picked from commit 89ca2ae)
|
🎉 Handy links: |
|
🎉 Handy links: |
|
follow link above to MS store download > Install > Launch > watch window crash within 10 seconds. |
|
@greg-hammond Can you file a new issue, with a /feedback link in it? I'm guessing whatever crash you're seeing isn't the same thing that was fixed by this PR 😉 |
I cannot for the life of me repro the original bug. I've got fonts with bad permissions SxS, I've tried installing a font twice, I've tried stopping the font cache service. No idea how to manually repro the original bug.
BUT theoretically, this function should never throw. So lets just switch this to a
LOG_IF_FAILED, and hope that this goes away?Unclear if this can get cherry-picked trivially to 1.8. Code's pretty trivial though so if we need another PR for that, it can be arranged.