Closed Bug 888445 Opened 6 years ago Closed 6 years ago

Only whitelist NVIDIA for SkiaGL canvas

Categories

(Core :: Canvas: 2D, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: snorp, Assigned: snorp)

Details

Attachments

(1 file)

We have a few test failures on PVR and Adreno with SkiaGL. For the first landing lets only whitelist NVIDIA and then fix the others later.
Sorry the patch is so hard to read. The indentation there was messed up and it was driving me insane.
Comment on attachment 769103 [details] [diff] [review]
Only use SkiaGL canvas on NVIDIA

Review of attachment 769103 [details] [diff] [review]:
-----------------------------------------------------------------

It is sad that we have to restrict Skia/GL only to NVIDIA. But another part of me rejoices that this underlines the usefulness of our separate-java-thread-querying-GL-info at startup on android...

::: content/canvas/src/CanvasRenderingContext2D.cpp
@@ +874,5 @@
> +        nsRefPtr<GLContext> glContext;
> +        nsCOMPtr<nsIGfxInfo> gfxInfo = do_GetService("@mozilla.org/gfx/info;1");
> +        nsString vendor;
> +
> +        if (!mForceSoftware && gfxInfo &&

Please move gfxInfo&& to a new line. Also, I don't suppose that gfxInfo is available at all on B2G (I am not aware of it being implemented there). Please confirm that it is intentional that this disables Skia/GL on B2G, and will continue to even if you check for Qualcomm below. We have little or no interest in blacklisting on B2G due to the lower variety of hw and drivers there. So I would rather prefer if you wrote this code so as to skip any blacklist check (i.e. wholesale whitelist) on B2G.

@@ +876,5 @@
> +        nsString vendor;
> +
> +        if (!mForceSoftware && gfxInfo &&
> +            NS_SUCCEEDED(gfxInfo->GetAdapterVendorID(vendor)) &&
> +            StringBeginsWith(vendor, NS_LITERAL_STRING("NVIDIA"))) {

After multi-line if() condition, please move { to a new line
Attachment #769103 - Flags: review?(bjacob) → review+
https://hg.mozilla.org/mozilla-central/rev/8e87e5a44607
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.