Closed Bug 899667 Opened 6 years ago Closed 6 years ago

Make layer borders more readable

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: nical, Assigned: nical)

Details

Attachments

(3 files, 1 obsolete file)

Layer borders right now are a bit hard to read especially on android because layers are split into a lot of tiles which makes it hard to see the separation between actual layers.

I think we need to:
 * allow/disallow tile borders independently of layer borders
 * make layer borders stand out next to tile borders (thicker line, brighter color, different opacity...)
Attached patch layer-borders.patch (obsolete) — Splinter Review
I pulled some of the changes from the patch in bug 877567 (without the exposing to devtools part), fixed the comments and cleaned up.

This patch adds two prefs "layers.draw-tile-borders" and "layers.draw-bigimage-borders" to visualize tiles and BigImage sections independently of layer borders (already existing "layer.draw-borders" pref).

layer borders are now 2px thick while tile borders and BigImage section borders are 1px thick, and the latter two are slightly more transparent.

This patch also moves the colors in Compositor::DrawDiagnostics so that the method interface becomes a bit more general (other per-layer debug tools should go there).
Attachment #783245 - Flags: review?(bgirard)
Layer border is quite confusing with all the colors types. Can you make a simple wiki page with a mapping of colors/style to what they represent? I just recently learned that blue/green are both thebes layer except that blue has component alpha.
(In reply to Benoit Girard (:BenWa) from comment #2)
> Layer border is quite confusing with all the colors types. Can you make a
> simple wiki page with a mapping of colors/style to what they represent? I
> just recently learned that blue/green are both thebes layer except that blue
> has component alpha.

Oh, I broke the component alpha specific color. Nice catch!

Yeah I'll add this info to a wiki page somewhere. The idea behind moving the colors to DrawDiagnostics is also that now they are defined at the same place (but yeah people should not need to go check the code to find out what color is what, although i don't know which of the wiki page or the actual code will have more visibility...)
Fixed the component alpha border color.

Btw, on the other bug you asked why using a constant integer rather than an enum. The problem with enums for bitfields is that you can't use bit operators without casting everywhere. I have the same problem with mfbt's EnumSet: you can't write DiagnosticTypes types = DIAGNOSTIC_FOO | DIAGNOSTIC_BAR; if DiagnosticFlags is an EnumSet.

instead you have to write DiagnosticFlags types = DiagnosticFlags(DIAGNOSTIC_FOO) | DiagnosticFlags(DIAGNOSTIC_BAR);
Or write: DiagnosticFlags types = DiagnosticFlags(DIAGNOSTIC_FOO, DIAGNOSTIC_FLAGS);

The latter is a bit less cumbersome but when you read it for the first time the first idea that pops is not "oh okay, that's just a bitfield" so I ended up using integers like we do in the rest of layers.
Attachment #783245 - Attachment is obsolete: true
Attachment #783245 - Flags: review?(bgirard)
Attachment #783739 - Flags: review?(bgirard)
Comment on attachment 783739 [details] [diff] [review]
Make layer borders more readable

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

::: gfx/layers/Compositor.cpp
@@ +57,5 @@
> +    lWidth = 1;
> +    opacity = 0.5;
> +    color.r *= 0.7;
> +    color.g *= 0.7;
> +    color.b *= 0.7;

Will this really be visible? I think we should look into putting text in the top level instead to communicate more complex cases.
Attachment #783739 - Flags: review?(bgirard) → review+
Ok looks great! We talked about, in Toronto atleast, drawing the layer visible rect a lighter color and the layer bounds a darker color so we can tell whether something is several layers or one discontinuous layer. It might be a bit confusing if the tile/big image border is also enabled. What do you think?
If we add what you propose we don't have to enable all the prefs at once, people would enable whatever border they are interested in for debuging (for instance webdevs will only care about layer bounds i suppose).
There is just so much info we can put on the screen at once.

Another thing that could be nice is to "flash" the border (white and then profressively fade to it's normal color, or just change to a random color like paint flashing) when the layer is created/reuploaded to have a feel of when these happen.
It would tell us for instance if we are recreating the same layer constantly.
Yes great idea. Feel free to land what you have now or do it in a follow up.
https://hg.mozilla.org/mozilla-central/rev/78c0cf01f34f
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.