Using the device scale factor for glyph positioning (second approach)

Using the device scale factor for glyph positioning (second approach)

The master bug is http://code.google.com/p/chromium/issues/detail?id=138101 .  The corresponding WebKit changes for this CL are here: https://bugs.webkit.org/show_bug.cgi?id=96137

Note that the changes in https://codereview.appspot.com/6488085 should also be included in this CL.

BUG=

[reed1, 2012-09-10 16:55:39]

I think this is correct, in that it should not affect clients that don't opt-in
with the new API.

Adding more weight to SkPaint is a shame, though maybe unavoidable in the
short-term. Is this seen as the best solution for here-on-out, or is this seen
as a stop-gap until we have the rest of the kinks worked out of subpixel
positioning?

[tdanderson, 2012-09-10 17:01:46]

On 2012/09/10 16:55:39, reed1 wrote:
> I think this is correct, in that it should not affect clients that don't
opt-in
> with the new API.
> 
> Adding more weight to SkPaint is a shame, though maybe unavoidable in the
> short-term. Is this seen as the best solution for here-on-out, or is this seen
> as a stop-gap until we have the rest of the kinks worked out of subpixel
> positioning?

This is seen as a short-term solution for the time being. We will want to move
to subpixel positioning once the kinks have been worked out.

[reed1, 2012-09-10 17:50:11]

(short-term) Good.

Lets wrap this in a build-flag, as we've done for some Android changes that we
hope to remove down the road as well.

e.g.

#ifdef SK_SUPPORT_HINTING_SCALE_FACTOR
...

That we callers can see that they should not build-up a dependency on this API.

[tdanderson, 2012-09-11 18:29:10]

On 2012/09/10 17:50:11, reed1 wrote:
> (short-term) Good.
> 
> Lets wrap this in a build-flag, as we've done for some Android changes that we
> hope to remove down the road as well.
> 
> e.g.
> 
> #ifdef SK_SUPPORT_HINTING_SCALE_FACTOR
> ...
> 
> That we callers can see that they should not build-up a dependency on this
API.

OK, Patch Set 4 uses the build flag as you suggested. Once this patch +
https://codereview.appspot.com/6488085 lands, we can turn this on in chromium
here: https://chromiumcodereview.appspot.com/10918175

[reed1, 2012-09-11 18:58:32]

(assuming you added the build-flag in SkPaint.h as well)
lgtm

[tdanderson, 2012-09-11 19:00:20]

On 2012/09/11 18:58:32, reed1 wrote:
> (assuming you added the build-flag in SkPaint.h as well)
> lgtm

I did, and I just added you as reviewer for
https://codereview.appspot.com/6488085/ . Can these two CLs be landed at the
same time?

[reed1, 2012-09-11 19:04:32]

On 2012/09/11 19:00:20, tdanderson wrote:
> On 2012/09/11 18:58:32, reed1 wrote:
> > (assuming you added the build-flag in SkPaint.h as well)
> > lgtm
> 
> I did, and I just added you as reviewer for
> https://codereview.appspot.com/6488085/ . Can these two CLs be landed at the
> same time?

Nope. Much better if you combine them into one CL. Just back up one directory
before you run 'gcl change foo' and you should get all the changes at once.

[tdanderson, 2012-09-11 20:27:03]

On 2012/09/11 19:04:32, reed1 wrote:
> On 2012/09/11 19:00:20, tdanderson wrote:
> > On 2012/09/11 18:58:32, reed1 wrote:
> > > (assuming you added the build-flag in SkPaint.h as well)
> > > lgtm
> > 
> > I did, and I just added you as reviewer for
> > https://codereview.appspot.com/6488085/ . Can these two CLs be landed at the
> > same time?
> 
> Nope. Much better if you combine them into one CL. Just back up one directory
> before you run 'gcl change foo' and you should get all the changes at once.

I don't think this will work for me since src/third_party/skia/src and
src/third_party/skia/include are two different repositories in my current setup
for some reason. Backing up to src/third_party/skia puts me in the chromium
repository. I'm trying to sort this out.

[reed1, 2012-09-11 20:54:12]

Ah!

You need to construct the CL from a clean Skia checkout, not from the
partial, embedded checkout that chrome's DEPS file creates.

On Tue, Sep 11, 2012 at 4:27 PM,  <tdanderson@chromium.org> wrote:
> On 2012/09/11 19:04:32, reed1 wrote:
>>
>> On 2012/09/11 19:00:20, tdanderson wrote:
>> > On 2012/09/11 18:58:32, reed1 wrote:
>> > > (assuming you added the build-flag in SkPaint.h as well)
>> > > lgtm
>> >
>> > I did, and I just added you as reviewer for
>> > https://codereview.appspot.com/6488085/ . Can these two CLs be
>
> landed at the
>>
>> > same time?
>
>
>> Nope. Much better if you combine them into one CL. Just back up one
>
> directory
>>
>> before you run 'gcl change foo' and you should get all the changes at
>
> once.
>
> I don't think this will work for me since src/third_party/skia/src and
> src/third_party/skia/include are two different repositories in my
> current setup for some reason. Backing up to src/third_party/skia puts
> me in the chromium repository. I'm trying to sort this out.
>
> https://codereview.appspot.com/6495089/

[tdanderson, 2012-09-11 23:32:29]

On 2012/09/11 20:54:12, reed1 wrote:
> Ah!
> 
> You need to construct the CL from a clean Skia checkout, not from the
> partial, embedded checkout that chrome's DEPS file creates.
> 
> On Tue, Sep 11, 2012 at 4:27 PM,  <mailto:tdanderson@chromium.org> wrote:
> > On 2012/09/11 19:04:32, reed1 wrote:
> >>
> >> On 2012/09/11 19:00:20, tdanderson wrote:
> >> > On 2012/09/11 18:58:32, reed1 wrote:
> >> > > (assuming you added the build-flag in SkPaint.h as well)
> >> > > lgtm
> >> >
> >> > I did, and I just added you as reviewer for
> >> > https://codereview.appspot.com/6488085/ . Can these two CLs be
> >
> > landed at the
> >>
> >> > same time?
> >
> >
> >> Nope. Much better if you combine them into one CL. Just back up one
> >
> > directory
> >>
> >> before you run 'gcl change foo' and you should get all the changes at
> >
> > once.
> >
> > I don't think this will work for me since src/third_party/skia/src and
> > src/third_party/skia/include are two different repositories in my
> > current setup for some reason. Backing up to src/third_party/skia puts
> > me in the chromium repository. I'm trying to sort this out.
> >
> > https://codereview.appspot.com/6495089/

OK, everything is finally in one place now:
https://codereview.appspot.com/6506099/