Closed Bug 1137921 Opened 7 years ago Closed 7 years ago

Use 9-patch for View shadows

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox39 fixed)

RESOLVED FIXED
Firefox 39
Tracking Status
firefox39 --- fixed

People

(Reporter: liuche, Assigned: liuche)

References

Details

Attachments

(4 files, 5 obsolete files)

No description provided.
Attached file shadow 9patch.zip (obsolete) —
Let's try these.
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.
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!
Flags: needinfo?(alam)
Sent files via IRC
Flags: needinfo?(alam)
Attached file shadow3_9patch_nolines.zip (obsolete) —
Attachment #8570725 - Attachment is obsolete: true
Leaving a note here we might want to explore using Android L's "elevation value" for some of these shadows
Note of ratios:

MDPI: 1
HDPI: 1.5
XHDPI: 2
XXHDPI: 3
Assignee: alam → liuche
Attached image 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.
(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.
Anthony, what do you think?
Flags: needinfo?(alam)
Flags: needinfo?(alam)
Flags: needinfo?(alam)
Shadow looks good! 

Doorhanger position needs to move up a dp or two I reckon but that's not in this bug :)
Flags: needinfo?(alam)
Summary: Experimental 9-patch for shadow → Use 9-patch for View shadows
Attached file MozReview Request: bz://1137921/liuche (obsolete) —
/r/4741 - Bug 1137921 - Use 9-patch for View shadows. r=margaret

Pull down this commit:

hg pull review -r d75b0fedc95a3b486bb8228a335fc365ceb6a1c7
Still have some tweaks to make - figuring out only some of the corners are rounded, improving the shadow for antlam.
Attachment #8573358 - Flags: review?(margaret.leibovic)
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 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.
Attachment #8573358 - Flags: review?(margaret.leibovic) → review+
Attachment #8573358 - Flags: review?(michael.l.comella)
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>?
Attachment #8573358 - Flags: review?(michael.l.comella) → review+
Blocks: 1140674
Corners become sharp (instead of rounded) when scrolling up. Filed follow-up bug 1140674.
https://hg.mozilla.org/integration/fx-team/rev/534e729ad1d7
Target Milestone: --- → Firefox 39
https://hg.mozilla.org/mozilla-central/rev/534e729ad1d7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Attachment #8589613 - Attachment description: Dropshadow commit revision → (38 rebase) Add dropshadow assets
Attachment #8589613 - Attachment is patch: true
Attachment #8573358 - Attachment is obsolete: true
Attachment #8619623 - Flags: review+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.