1323157 - Rename HasNoFilterOrHasValidFilter as HasNoOrValidFilter

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[u459114]

File this follow up base on Robert's comment in bug 1322330 comment 26:
create a follow up bug to rename HasNoFilterOrHasValidFilter to be consistent with the other two i.e. to become HasNoOrHasValidFilter.

Comment 1

[u459114]

Attachment: Bug 1323157 - Rename HasNoFilterOrHasValidFilter as HasNoOrValidFilter.

Review commit: https://reviewboard.mozilla.org/r/98380/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98380/

Comment 2

[u459114]

Comment on attachment 8818233 [details]
Bug 1323157 - Rename HasNoFilterOrHasValidFilter as HasNoOrValidFilter.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98380/diff/1-2/

Comment 3

[Robert Longson [:longsonr]]

Comment on attachment 8818233 [details]
Bug 1323157 - Rename HasNoFilterOrHasValidFilter as HasNoOrValidFilter.

https://reviewboard.mozilla.org/r/98380/#review98816

Comment 4

[Robert Longson [:longsonr]]

Comment on attachment 8818233 [details]
Bug 1323157 - Rename HasNoFilterOrHasValidFilter as HasNoOrValidFilter.

https://reviewboard.mozilla.org/r/98380/#review98820

::: layout/painting/nsDisplayList.cpp
(Diff revision 2)
> +             "By getting here, we must have valid CSS filters.");
> +
>    ContainerLayerParameters newContainerParameters = aContainerParameters;
> -  if (effectProperties.HasValidFilter()) {
> -    newContainerParameters.mDisableSubpixelAntialiasingInDescendants = true;
> +  newContainerParameters.mDisableSubpixelAntialiasingInDescendants = true;
> -  }

Why can this be removed? I think the point of this is to disable antialiasing when text is filtered because it tends to look odd. See bug 812795 for more details

Comment 5

[Robert Longson [:longsonr]]

Comment on attachment 8818233 [details]
Bug 1323157 - Rename HasNoFilterOrHasValidFilter as HasNoOrValidFilter.

https://reviewboard.mozilla.org/r/98380/#review98822

Comment 6

[u459114]

Comment on attachment 8818233 [details]
Bug 1323157 - Rename HasNoFilterOrHasValidFilter as HasNoOrValidFilter.

https://reviewboard.mozilla.org/r/98380/#review98820

> Why can this be removed? I think the point of this is to disable antialiasing when text is filtered because it tends to look odd. See bug 812795 for more details

I did not remove code of disabling antialiasing, all I did here was removing if statement.
1. This is an nsDisplayFilter(I splitted nsDisplaySVGEffects into nsDisplayMask and nsDisplayFilter), so we must have CSS filter effect(valid or invalid).
2. I check effectProperties.HasInvalidFilter() several lines above. If we pass that test and reach #7514, we must have valid CSS filter. So we should always disable antialiasing

Comment 7

[Robert Longson [:longsonr]]

Comment on attachment 8818233 [details]
Bug 1323157 - Rename HasNoFilterOrHasValidFilter as HasNoOrValidFilter.

https://reviewboard.mozilla.org/r/98380/#review98978

Comment 8

[Robert Longson [:longsonr]]

Comment on attachment 8818233 [details]
Bug 1323157 - Rename HasNoFilterOrHasValidFilter as HasNoOrValidFilter.

https://reviewboard.mozilla.org/r/98380/#review98820

> I did not remove code of disabling antialiasing, all I did here was removing if statement.
> 1. This is an nsDisplayFilter(I splitted nsDisplaySVGEffects into nsDisplayMask and nsDisplayFilter), so we must have CSS filter effect(valid or invalid).
> 2. I check effectProperties.HasInvalidFilter() several lines above. If we pass that test and reach #7514, we must have valid CSS filter. So we should always disable antialiasing

OK, I'll buy that explanation :-)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d28fe1daf68
Rename HasNoFilterOrHasValidFilter as HasNoOrValidFilter. r=longsonr+218550

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2d28fe1daf68