1211324 - Remove GraphicsFilter

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Nicholas Nethercote [inactive]]

It should be possible to remove GraphicsFilter and replace all its uses with gfx::Filter. Though there's some weirdness that needs to be resolved first...

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Remove BILINEAR and GAUSSIAN filter constants

These aren't used meaningfully.

Comment 2

[Nicholas Nethercote [inactive]]

The conversions from GraphicsFilter to gfx::Filter and back (in ToFilter() and ThebesFilter()) are screwy:

- FILTER_NEAREST    <--> POINT
- FILTER_GOOD        --> GOOD   --> FILTER_BEST (not FILTER_GOOD!)
- FILTER_{FAST,BEST} --> LINEAR --> FILTER_BEST
- FILTER_FAST        --> LINEAR --> FILTER_BEST

Without knowing anything of the broader context, one way to fix it up might be the following:

- Make FILTER_GOOD <--> GOOD.

- Remove FILTER_FAST and replace it throughout with FILTER_BEST; even though
  that is conceptually weird it's what the current code effectively does.

Then there would be a 1-to-1 correspondence between GraphicsFilter and gfx::Filter and removing the former would be easy.

mattwoodrow: please tell me what I have gotten wrong about this :)

Comment 3

[Matt Woodrow (:mattwoodrow)]

I agree, that sounds like the right plan of action.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Make gfx::FILTER::GOOD convert to GraphicsFilter::FILTER_GOOD

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Remove GraphicsFilter::FILTER_FAST and replace it with FILTER_BEST

This may sound like an odd change but it's what the current code effectively
already does due to the way ToFilter() and ThebesFilter() are defined.

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Replace GraphicsFilter constants with gfx::Filter equivalents

The conversion is as follows:

- GraphicsFilter::FILTER_NEAREST == gfx::Filter::POINT
- GraphicsFilter::FILTER_GOOD    == gfx::Filter::GOOD
- GraphicsFilter::FILTER_BEST    == gfx::Filter::LINEAR

Also typedef GraphicsFilter to gfx::Filter; this will be removed in the next
patch.

These changes mean ToFilter() and ThebesFilter() are no longer needed.

Comment 7

[Nicholas Nethercote [inactive]]

(In reply to Nicholas Nethercote [:njn] from comment #6)
> Created attachment 8670045 [details] [diff] [review]
> (part 4) - Replace GraphicsFilter constants with gfx::Filter equivalents
> 

After this patch, in nsLayoutUtils::GetGraphicsFilterForFrame() we end up using Filter::LINEAR for both NS_STYLE_IMAGE_RENDERING_OPTIMIZESPEED and NS_STYLE_IMAGE_RENDERING_OPTIMIZEQUALITY.

Maybe Filter::POINT would be better for the OPTIMIZESPEED case?

Comment 8

[Nicholas Nethercote [inactive]]

Attachment: (part 5) - Remove GraphicsFilter and gfxGraphicsFilter

Comment 9

[Matt Woodrow (:mattwoodrow)]

(In reply to Nicholas Nethercote [:njn] from comment #7)
> (In reply to Nicholas Nethercote [:njn] from comment #6)
> > Created attachment 8670045 [details] [diff] [review]
> > (part 4) - Replace GraphicsFilter constants with gfx::Filter equivalents
> > 
> 
> After this patch, in nsLayoutUtils::GetGraphicsFilterForFrame() we end up
> using Filter::LINEAR for both NS_STYLE_IMAGE_RENDERING_OPTIMIZESPEED and
> NS_STYLE_IMAGE_RENDERING_OPTIMIZEQUALITY.
> 
> Maybe Filter::POINT would be better for the OPTIMIZESPEED case?

That sounds like the right thing to do. It's possible that it'll change rendering and make reftests fail, will need a try run.

Comment 10

[Nicholas Nethercote [inactive]]

Thank you for the fast reviews.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7379c6bce682
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b027441cbee
https://hg.mozilla.org/integration/mozilla-inbound/rev/3897da20e42d
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7cd4d58ea9e
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1fe3d531b96

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7379c6bce682
https://hg.mozilla.org/mozilla-central/rev/6b027441cbee
https://hg.mozilla.org/mozilla-central/rev/3897da20e42d
https://hg.mozilla.org/mozilla-central/rev/a7cd4d58ea9e
https://hg.mozilla.org/mozilla-central/rev/e1fe3d531b96