Closed Bug 1348584 Opened 3 years ago Closed 3 years ago

Windows ClearType Enhanced Contrast: 300 causes fatal error: "assert(0 <= c && c <= SK_Scalar1)"

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: jorgk, Assigned: mchang)

References

Details

Attachments

(1 file)

I'm running a Thunderbird debug build, pretty much current at time of writing.

That build worked earlier today, but now I'm getting a crash:

c:\mozilla-source\comm-central\mozilla\gfx\skia\skia\src\core\SkScalerContext.h:77: fatal error: "assert(0 <= c && c <= SK_Scalar1)"
Abort from sk_abort
Hit MOZ_CRASH() at c:/mozilla-source/comm-central/mozilla/memory/mozalloc/mozalloc_abort.cpp:33

So I printed out the value of c and SK_Scalar1:
    void setContrast(SkScalar c) {
        printf("Skia: %f %f\n", c, SK_Scalar1);
        SkASSERT(0 <= c && c <= SK_Scalar1);
        fContrast = SkScalarRoundToInt(c * ((1 << 8) - 1));
    }

Result on two calls, the second crashes:
Skia: 0.500000 1.000000
Skia: 3.000000 1.000000

I have no idea what's going on here since I'm not familiar with graphics.

This is a Windows 10 x64 PC with a x64 build of Thunderbird.

I can get you a better stack if you want, this is called from
void DWriteFontTypeface::onFilterRec(SkScalerContext::Rec* rec)
    IDWriteRenderingParams* params = GetDwriteRenderingParams(ForceGDI());
    SkASSERT(params);
    rec->setContrast(params->GetEnhancedContrast());

I will only have access to this machine for one week, so if you want me to experiment, you need to ask now.
OK, looking at the troubleshooting information, I see:
ClearType Parameters	Gamma: 2.2 Pixel Structure: RGB ClearType Level: 50 Enhanced Contrast: 300

Please don't ask how I managed to get those settings ;-)
Summary: fatal error: "assert(0 <= c && c <= SK_Scalar1) → Windows ClearType Enhanced Contrast: 300 causes fatal error: "assert(0 <= c && c <= SK_Scalar1)"
HKEY_CURRENT_USER\SOFTWARE\Microsoft\Avalon.Graphics\DISPLAY1 is set to 300. No idea how that happened, but it shouldn't crash, not even in debug mode.
I encountered exactly same startup crash in Firefox nightly.

And my EnhancedContrastLevel was set to 300, too.

The crash can be fixed by setting EnhancedContrastLevel to 0, but it should not happen.

I think it is a regression introduced quite recently because I came across this issue yesterday after a clobber build.
Lee and Mason: Any chance of getting this fixed, it's affecting developers.
Flags: needinfo?(mchang)
Flags: needinfo?(lsalzman)
Assignee: nobody → mchang
Blocks: 1345222
Status: NEW → ASSIGNED
Flags: needinfo?(mchang)
Flags: needinfo?(lsalzman)
See Also: → 1349220
Duplicate of this bug: 1349220
This will revert us to the behavior pre- bug 1345222. Essentially, if the contrast isn't between 0-1 and we have skia, default back to 1.0.
Attachment #8850095 - Flags: review?(VYV03354)
Comment on attachment 8850095 [details] [diff] [review]
Limit contrast to 0-1 if we have a skia backend

This works for me, thanks. No one asked me for feedback, but since I filed the bug, I might was well show my appreciation. The warning shows up once, which is fine.
Attachment #8850095 - Flags: feedback+
(In reply to Jorg K (GMT+1) from comment #7)
> Comment on attachment 8850095 [details] [diff] [review]
> Limit contrast to 0-1 if we have a skia backend
> 
> This works for me, thanks. No one asked me for feedback, but since I filed
> the bug, I might was well show my appreciation. The warning shows up once,
> which is fine.

Thanks for filing and testing!
Comment on attachment 8850095 [details] [diff] [review]
Limit contrast to 0-1 if we have a skia backend

I found a document:
https://msdn.microsoft.com/en-us/library/windows/desktop/dd371290(v=vs.85).aspx
> Remarks
> 
> Enhanced contrast is the amount to increase the darkness of text, and
> typically ranges from 0 to 1. Zero means no contrast enhancement.

although it is not a hard requirement for DWrite backend.
Attachment #8850095 - Flags: review?(VYV03354) → review+
We will need uplift to whatever branches that bug 1345222 uplifted to.
Pushed by mchang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b95938854605
Default to 1.0 contrast for Skia backends if custom contrast dwrite param isn't supported by skia. r=emk
https://hg.mozilla.org/mozilla-central/rev/b95938854605
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment on attachment 8850095 [details] [diff] [review]
Limit contrast to 0-1 if we have a skia backend

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1345222
[User impact if declined]: Debug builds will assert crash. Users with custom cleartype contrast parameters will see no text or very colorful pixels shaped somewhat like text.
[Is this code covered by automated tests?]: No, manually tested and verified.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: No. 
[Why is the change risky/not risky?]: this reverts us to the behavior for skia pre bug 1345222.
[String changes made/needed]: None
Attachment #8850095 - Flags: approval-mozilla-beta?
Attachment #8850095 - Flags: approval-mozilla-aurora?
Comment on attachment 8850095 [details] [diff] [review]
Limit contrast to 0-1 if we have a skia backend

Fix an assert crash. Aurora54+ & Beta53+.
Attachment #8850095 - Flags: approval-mozilla-beta?
Attachment #8850095 - Flags: approval-mozilla-beta+
Attachment #8850095 - Flags: approval-mozilla-aurora?
Attachment #8850095 - Flags: approval-mozilla-aurora+
Comment on attachment 8850095 [details] [diff] [review]
Limit contrast to 0-1 if we have a skia backend

we need this in esr52 along with bug 1345222
Attachment #8850095 - Flags: approval-mozilla-esr52+
I am experiencing the Bug just in a current trunk (TB & FF 56.0a1) build.

| s:\Projects\mozi\comm-central\mozilla\gfx\skia\skia\src\core\SkScalerContext.h:76: fatal error: "assert(0 <= c && c <= SK_Scalar1)"
| Abort from sk_abort
| Hit MOZ_CRASH() at s:/Projects/mozi/comm-central/mozilla/memory/mozalloc/mozalloc_abort.cpp:33
| [GPU 8164] WARNING: Shutting down GPU process early due to a crash!: file s:/Projects/mozi/comm-central/mozilla/gfx/ipc/GPUParent.cpp, line 399

Is this fix incomplete? Or is it a regression?

I traced it back to this call to: "params->GetEnhancedContrast()"
<https://dxr.mozilla.org/comm-central/source/mozilla/gfx/thebes/gfxDWriteFonts.cpp#718>

This seems to bypass your fix.
(In reply to Alfred Peters from comment #20)
> Is this fix incomplete? Or is it a regression?

A regression of Bug 1376026
Depends on: 1383817
You need to log in before you can comment on or make changes to this bug.