Page MenuHomePhabricator

Fix face-centering to only apply vertical offset (instead of zooming)
Closed, ResolvedPublic


Steps to reproduce

  1. Open "Spiderman in film" on EN Wiki
  2. Wait until lead image downloads

Expected results

Both actors' faces are centered, as in original image:

Actual results

Only one face is centered (horizontally and vertically) and zoomed to the point of blurriness:

Dev Notes

This is due to oversight in refactoring lead image work to use -{UIImage contentsRect] which also change horizontal offset & width. Should be a quick fix to change the API to "vertically center" an image and preserve original width w/o applying horizontal offset.

Event Timeline

BGerstle-WMF updated the task description. (Show Details)
BGerstle-WMF raised the priority of this task from to Needs Triage.
BGerstle-WMF added a subscriber: BGerstle-WMF.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 5 2015, 5:57 PM

@JMinor some background: @Mhurd told me about this and I was able to fix it locally real quick, but was in the middle of something else so didn't integrate it. I just filed a ticket to capture the bug, but I can submit a patch to fix this quickly.

BGerstle-WMF triaged this task as Low priority.Oct 6 2015, 6:08 PM
BGerstle-WMF set Security to None.

Checked in alpha 5.0.0 (406) with iPad mini iOS 8.2

vs 4.1.7(171)

Note: 'Nellie Kim' and 'Viceregal consort of Canada' lead images still have centering issues.

JMinor closed this task as Resolved.Nov 18 2015, 9:09 PM

I think the new face centering is an improvement, but there are definitely cases it doesn't handle well. I think we'd need to invest time in a systematic sampling/eval/error reduction process to make it any better, and we still just might be bumping up against platform limits.

Could be a great community contributor or research-y project, but good enuf for now.