Only coerce interpolation mode returned from computeInterpolationQuality

Only coerce interpolation mode returned from computeInterpolationQuality

This makes the interpolation mode remain as InterpolationNone if the
'printing' flag is set, which is in line with the documentation for said
flag.

This behavior was added in: 
  http://trac.webkit.org/changeset/117187
which did not mention the printing case.

This is a first step towards separating "per context" and "per image"
interpolation settings/adaption.

BUG=492794
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=198934

[fs, 2015-07-14 15:20:00]

fs@opera.com changed reviewers:
+ fmalita@chromium.org, senorblanco@chromium.org

[fs, 2015-07-14 15:20:00]

This has been like this for a long time (see description for commit introducing
it; reviewed by senorblanco), which makes it a bit scary... If you know of
anyone particularly knowledgeable of the printing cases (or know of tests for
printing output), please do add them.

[f(malita), 2015-07-14 16:11:36]

fmalita@chromium.org changed reviewers:
+ halcanary@chromium.org

[f(malita), 2015-07-14 16:12:11]

I think this makes sense if we are to trust the comments, but is that truly the
case?

    // Returns if the context is a printing context instead of a display
    // context. Bitmap shouldn't be resampled when printing to keep the best
    // possible quality.
    bool printing() const { return m_printing; }

Won't kNone_SkFilterQuality make scaled images look horrible in PDFs?

[fs, 2015-07-14 16:39:17]

On 2015/07/14 16:12:11, f(malita) wrote:
> I think this makes sense if we are to trust the comments, but is that truly
the
> case?
> 
>     // Returns if the context is a printing context instead of a display
>     // context. Bitmap shouldn't be resampled when printing to keep the best
>     // possible quality.
>     bool printing() const { return m_printing; }
> 
> Won't kNone_SkFilterQuality make scaled images look horrible in PDFs?

Quite possible. I looked at the SkPDF implementation though and didn't see it
emitting "Interpolate" for the images regardless though. My own understanding of
PDF is somewhat (old and) limited though.

[f(malita), 2015-07-14 16:42:20]

On 2015/07/14 16:12:11, f(malita) wrote:
> I think this makes sense if we are to trust the comments, but is that truly
the
> case?
> 
>     // Returns if the context is a printing context instead of a display
>     // context. Bitmap shouldn't be resampled when printing to keep the best
>     // possible quality.
>     bool printing() const { return m_printing; }
> 
> Won't kNone_SkFilterQuality make scaled images look horrible in PDFs?

From a quick side-chat with Hal, it seems the filter level doesn't play much of
a role with PDF (we embed the original image, and let the viewer/renderer scale
it as it sees fit).

(we might be able to further simplify this code if resampling doesn't matter at
all when printing, but that can be another CL)


So this LGTM, but senorblanco should also take a look.

[fs, 2015-07-14 17:30:07]

On 2015/07/14 16:42:20, f(malita) wrote:
> On 2015/07/14 16:12:11, f(malita) wrote:
> > I think this makes sense if we are to trust the comments, but is that truly
> the
> > case?
> > 
> >     // Returns if the context is a printing context instead of a display
> >     // context. Bitmap shouldn't be resampled when printing to keep the best
> >     // possible quality.
> >     bool printing() const { return m_printing; }
> > 
> > Won't kNone_SkFilterQuality make scaled images look horrible in PDFs?
> 
> From a quick side-chat with Hal, it seems the filter level doesn't play much
of
> a role with PDF (we embed the original image, and let the viewer/renderer
scale
> it as it sees fit).

Right, that's half what I expected.

> (we might be able to further simplify this code if resampling doesn't matter
at
> all when printing, but that can be another CL)

I've planned some follow-ups for this, which may or may not do what you're
thinking of. Do feel free to share your thoughts though. Right now I'm thinking
that I want t try and drop the GC 'printing' flag entirely (or make the printing
state more explicit via state configuration.)

[Stephen White, 2015-07-14 23:18:47]

Personally, I think we should just get rid of this kludge entirely, and respect
InterpolationNone from computeInterpolationQuality, and see what breaks. I'm
pretty sure it was a real bug that we discovered but avoided fixing back when
rebaselining was much harder.

But LGTM to land just this if you'd prefer.

[fs, 2015-07-15 07:57:19]

On 2015/07/14 23:18:47, Stephen White wrote:
> Personally, I think we should just get rid of this kludge entirely, and
respect
> InterpolationNone from computeInterpolationQuality, and see what breaks. I'm
> pretty sure it was a real bug that we discovered but avoided fixing back when
> rebaselining was much harder.

Yes, I'd like to remove that to - in due time (always coercing away one of the
three values that function returns make it cloe to deadish). I much prefer baby
steps though. See if I get to that before my vacation knocks on the door.

[fs, 2015-07-15 07:57:23]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2015-07-15 07:57:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1237083004/1

[commit-bot: I haz the power, 2015-07-15 08:02:47]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198934