fix paths and remove dead code

fix paths and remove dead code

More Skia driver cleanup.

Fix the GN build for Skia.
Remove unused functions from header.
Remove agg setup that is not required.

Change path construction to use the SkCanvas matrix to position the path
rather than converting the points directly.

Draw stroked paths using Skia rather than generated the filled path.
Pin the minimum stroke width to 1 px in device space to mimic PDF's stroke
dropout control.

Factor out flipped and non-flipped matrices.

Add some debugging code.

Set the bitmap filter quality to high. This helps a lot with 1 bit sources.

R=dsinclair@chromium.org, dsinclair, tsepez

Committed: https://pdfium.googlesource.com/pdfium/+/59b3a481fee140ed2508916e02addb8f3959ec7b

[caryclark, 2016-03-17 14:22:18]

Description was changed from

==========
test git cl upload

BUG=
==========

to

==========
fix paths and remove dead code

More Skia driver cleanup.

Fix the GN build for Skia.
Remove unused functions from header.
Remove agg setup that is not required.

Change path construction to use the SkCanvas matrix to position the path rather
than converting the points directly.

Draw stroked paths using Skia rather than generated the filled path.

Factor out flipped and non-flipped matrices.

Add some debugging code.

Set the bitmap filter quality to high. This helps a lot with 1 bit sources.

R=dsinclair,tsepez
==========

[caryclark, 2016-03-17 14:22:18]

caryclark@google.com changed reviewers:
+ dsinclair@chromium.org, tsepez@chromium.org

[caryclark, 2016-03-17 14:24:23]

Description was changed from

==========
fix paths and remove dead code

More Skia driver cleanup.

Fix the GN build for Skia.
Remove unused functions from header.
Remove agg setup that is not required.

Change path construction to use the SkCanvas matrix to position the path rather
than converting the points directly.

Draw stroked paths using Skia rather than generated the filled path.

Factor out flipped and non-flipped matrices.

Add some debugging code.

Set the bitmap filter quality to high. This helps a lot with 1 bit sources.

R=dsinclair,tsepez
==========

to

==========
fix paths and remove dead code

More Skia driver cleanup.

Fix the GN build for Skia.
Remove unused functions from header.
Remove agg setup that is not required.

Change path construction to use the SkCanvas matrix to position the path
rather than converting the points directly.

Draw stroked paths using Skia rather than generated the filled path.
Pin the minimum stroke width to 1 px in device space to mimic PDF's stroke 
dropout control.

Factor out flipped and non-flipped matrices.

Add some debugging code.

Set the bitmap filter quality to high. This helps a lot with 1 bit sources.

R=dsinclair,tsepez
==========

[dsinclair, 2016-03-17 14:39:31]

lgtm w/ nit

https://codereview.chromium.org/1806843002/diff/20001/core/fxge/skia/fx_skia_...
File core/fxge/skia/fx_skia_device.cpp (right):

https://codereview.chromium.org/1806843002/diff/20001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.cpp:131: }
nit: no {}'s on single line body

[caryclark, 2016-03-17 15:52:51]

https://codereview.chromium.org/1806843002/diff/20001/core/fxge/skia/fx_skia_...
File core/fxge/skia/fx_skia_device.cpp (right):

https://codereview.chromium.org/1806843002/diff/20001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.cpp:131: }
On 2016/03/17 14:39:31, dsinclair wrote:
> nit: no {}'s on single line body

Done.

[caryclark, 2016-03-17 16:00:42]

Description was changed from

==========
fix paths and remove dead code

More Skia driver cleanup.

Fix the GN build for Skia.
Remove unused functions from header.
Remove agg setup that is not required.

Change path construction to use the SkCanvas matrix to position the path
rather than converting the points directly.

Draw stroked paths using Skia rather than generated the filled path.
Pin the minimum stroke width to 1 px in device space to mimic PDF's stroke 
dropout control.

Factor out flipped and non-flipped matrices.

Add some debugging code.

Set the bitmap filter quality to high. This helps a lot with 1 bit sources.

R=dsinclair,tsepez
==========

to

==========
fix paths and remove dead code

More Skia driver cleanup.

Fix the GN build for Skia.
Remove unused functions from header.
Remove agg setup that is not required.

Change path construction to use the SkCanvas matrix to position the path
rather than converting the points directly.

Draw stroked paths using Skia rather than generated the filled path.
Pin the minimum stroke width to 1 px in device space to mimic PDF's stroke
dropout control.

Factor out flipped and non-flipped matrices.

Add some debugging code.

Set the bitmap filter quality to high. This helps a lot with 1 bit sources.

R=dsinclair@chromium.org, dsinclair, tsepez

Committed:
https://pdfium.googlesource.com/pdfium/+/59b3a481fee140ed2508916e02addb8f3959...
==========

[caryclark, 2016-03-17 16:00:44]

Committed patchset #3 (id:40001) manually as
59b3a481fee140ed2508916e02addb8f3959ec7b (presubmit successful).

[Tom Sepez, 2016-03-17 16:05:51]

lgtm

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
File core/fxge/skia/fx_skia_device.cpp (right):

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.cpp:40: printf("(%g,%g,%g) (%g,%g,%g)
(%g,%g,%g)\n", m[0], m[1], m[2], m[3], m[4], m[5], m[6],
nit: 80 cols.

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.cpp:136: SkTMin(deviceUnits[0].length(),
deviceUnits[1].length()));
nit: ditto.  git cl format should have fixed these for us?

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.cpp:206: static SkMatrix ToSkMatrix(const
CFX_Matrix& m) {
nit:  an we put these up top in the file with the other statics?

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.cpp:208: skMatrix.setAll(m.a, m.b, m.e, m.c, m.d,
m.f, 0, 0, 1);
nit: Pity SkMatrix doesn't have a constructor of this form, writing the function
body as the one-liner
  return SkMatrix(m.a, m.b, m.e, m.c, m.d, m.f, 0, 0, 1);
would then be possible.

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.cpp:303: SkMatrix skMatrix =
ToSkMatrix(*pObject2Device);
nit: local not needed.

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
File core/fxge/skia/fx_skia_device.h (right):

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.h:132: void PaintStroke(SkPaint* spaint, const
CFX_GraphStateData* pGraphState, const SkMatrix& matrix);
nit: 80 cols.

[caryclark, 2016-03-18 18:22:13]

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
File core/fxge/skia/fx_skia_device.cpp (right):

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.cpp:40: printf("(%g,%g,%g) (%g,%g,%g)
(%g,%g,%g)\n", m[0], m[1], m[2], m[3], m[4], m[5], m[6],
On 2016/03/17 16:05:50, Tom Sepez wrote:
> nit: 80 cols.

Done.

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.cpp:136: SkTMin(deviceUnits[0].length(),
deviceUnits[1].length()));
On 2016/03/17 16:05:50, Tom Sepez wrote:
> nit: ditto.  git cl format should have fixed these for us?

Done.

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.cpp:206: static SkMatrix ToSkMatrix(const
CFX_Matrix& m) {
On 2016/03/17 16:05:50, Tom Sepez wrote:
> nit:  an we put these up top in the file with the other statics?

Done.

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.cpp:208: skMatrix.setAll(m.a, m.b, m.e, m.c, m.d,
m.f, 0, 0, 1);
On 2016/03/17 16:05:50, Tom Sepez wrote:
> nit: Pity SkMatrix doesn't have a constructor of this form, writing the
function
> body as the one-liner
>   return SkMatrix(m.a, m.b, m.e, m.c, m.d, m.f, 0, 0, 1);
> would then be possible. 

Would you like a bug filed to request this feature? Or, are you asking for
something else?

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.cpp:303: SkMatrix skMatrix =
ToSkMatrix(*pObject2Device);
On 2016/03/17 16:05:50, Tom Sepez wrote:
> nit: local not needed.

While that's strictly true, it is easier to debug if stepping over this line
sets the matrix allowing inline examination of the values. Is this is a
performance concern, or a readability concern?

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
File core/fxge/skia/fx_skia_device.h (right):

https://codereview.chromium.org/1806843002/diff/40001/core/fxge/skia/fx_skia_...
core/fxge/skia/fx_skia_device.h:132: void PaintStroke(SkPaint* spaint, const
CFX_GraphStateData* pGraphState, const SkMatrix& matrix);
On 2016/03/17 16:05:50, Tom Sepez wrote:
> nit: 80 cols.

Done.