1137921 - Use 9-patch for View shadows

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: shadow 9patch.zip

Let's try these.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Note from discussion w/ liuche: we should be careful about how much overdraw we perform by using 9-patches, rather than just drawing where we explicitly need a shadow.

Comment 3

[Chenxia Liu [:liuche]]

Anthony, can you give me these 9-patches with a ring of 1px white around the edge? The transparency at the edges is giving me problems building.

Also, if it's possible, we should ideally make the center transparent as well!

Comment 4

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Sent files via IRC

Comment 5

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: shadow3_9patch_nolines.zip

Comment 6

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Leaving a note here we might want to explore using Android L's "elevation value" for some of these shadows

Comment 7

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Note of ratios:

MDPI: 1
HDPI: 1.5
XHDPI: 2
XXHDPI: 3

Comment 8

[Chenxia Liu [:liuche]]

Attachment: Screenshot: overdraw

@mcomella: I tried out the transparent centers, and it doesn't look like it makes any difference in overdraw, so we'll just drop that.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Chenxia Liu [:liuche] from comment #8)
> Created attachment 8571697 [details]
> Screenshot: overdraw
> 
> @mcomella: I tried out the transparent centers, and it doesn't look like it
> makes any difference in overdraw, so we'll just drop that.

I see some areas of RED in that screenshot. Might be worth filing followup bugs.

Comment 10

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: shadow4_9patch_nolines.zip

Comment 11

[Chenxia Liu [:liuche]]

Attachment: Screenshot: rectangular shadow + 4dp padding

Anthony, what do you think?

Comment 12

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Shadow looks good! 

Doorhanger position needs to move up a dp or two I reckon but that's not in this bug :)

Comment 13

[Chenxia Liu [:liuche]]

Attachment: MozReview Request: bz://1137921/liuche

/r/4741 - Bug 1137921 - Use 9-patch for View shadows. r=margaret

Pull down this commit:

hg pull review -r d75b0fedc95a3b486bb8228a335fc365ceb6a1c7

Comment 14

[Chenxia Liu [:liuche]]

Still have some tweaks to make - figuring out only some of the corners are rounded, improving the shadow for antlam.

Comment 15

[Chenxia Liu [:liuche]]

Comment on attachment 8573358 [details]
MozReview Request: bz://1137921/liuche

/r/4741 - Bug 1137921 - Use 9-patch for View shadows. r=margaret

Pull down this commit:

hg pull review -r fcd00525a637b6656b0253df108687fa029db5a9

Comment 16

[:Margaret Leibovic]

Comment on attachment 8573358 [details]
MozReview Request: bz://1137921/liuche

https://reviewboard.mozilla.org/r/4739/#review3915

This looks reasonable to me, but I'd also like mcomella to take a look because he has more context than me around these shadows.

Comment 17

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8573358 [details]
MozReview Request: bz://1137921/liuche

https://reviewboard.mozilla.org/r/4739/#review3997

(Apparently I can't comment on specific lines) Regarding the shadow, it appears to add a layer of overdraw. However, given that the alternative approach I can think of (drawing the shadow by hand in onDraw) adds a lot of complexity (especially with rounded corners), this seems like a good solution if there are no noticeable perf regressions.

On a different note, any reason to have <LinearLayout> <ScrollView> <LinearLayout>? Why not <ScrollView> <LinearLayout>?

Comment 18

[Chenxia Liu [:liuche]]

Corners become sharp (instead of rounded) when scrolling up. Filed follow-up bug 1140674.

Comment 19

[Chenxia Liu [:liuche]]

https://hg.mozilla.org/integration/fx-team/rev/534e729ad1d7

Comment 20

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/534e729ad1d7

Comment 21

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: (38 rebase) Add dropshadow assets

Comment 22

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: (38 rebase) Add Fennec color palette

Comment 24

[Chenxia Liu [:liuche]]

Attachment: MozReview Request: Bug 1137921 - Use 9-patch for View shadows. r=margaret