[Aura] Translate web contents area to nearest physical pixel boundary

[Aura] Translate web contents area to nearest physical pixel boundary

Views positions all views, including the views::WebView containing the
aura::Window that contains the content::RenderWidgetHostViewAura, to integral
DIP positions.  With a non-integral device scale factor, this could position
the RWHVA in between physical pixel boundaries which results in filtering the
composited output from the renderer. This looks blurry and bad.

This patch computes the delta to the top-left-most integer physical pixel inside the
desired bounds and translates the web contents layer by that amount. This means that
the top left corner of the web contents shows up at an exact physical pixel
and thus the web contents is not filtered. This means that some amount on the bottom
or right edge of the web contents area may be clipped, but this amount is always less
that one DIP.

BUG=370074, 378620
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274686

[Ben Goodger (Google), 2014-06-03 06:21:35]

Do you know if this issue generally affects all aura windows, or is it just the
content area?

[danakj, 2014-06-03 10:15:43]

Could we do something more general in the compositor where we know the screen
space position of each quad, and could shift the uvrect to compensate for the
positions?

[jamesr, 2014-06-03 17:13:01]

On 2014/06/03 06:21:35, Ben Goodger (Google) wrote:
> Do you know if this issue generally affects all aura windows, or is it just
the
> content area?

It potentially impacts all aura windows, but I'm not sure of another case where
it would matter.  We can see if we get more reports about bits of UI being
blurry.

[jamesr, 2014-06-03 17:18:04]

On 2014/06/03 10:15:43, danakj (OOO_back_june_6) wrote:
> Could we do something more general in the compositor where we know the screen
> space position of each quad, and could shift the uvrect to compensate for the
> positions?

No, because the compositor doesn't have enough context about what the
application is trying to do for a given layer/quad.  The decision to
translate+clip the content out has to be made at a higher level that's aware of
the context (such as here where we know that we're shifting web contents around
inside a frame).  For an arbitrary DelegatedRendererLayer, the compositor has no
idea if the contents should be snapped+clipped or filtered or something else -
consider the case where the delegated content is from an iframe with a CSS
animation or scale being applied.

[Ben Goodger (Google), 2014-06-03 20:49:50]

On 2014/06/03 17:13:01, jamesr wrote:
> On 2014/06/03 06:21:35, Ben Goodger (Google) wrote:
> > Do you know if this issue generally affects all aura windows, or is it just
> the
> > content area?
> 
> It potentially impacts all aura windows, but I'm not sure of another case
where
> it would matter.  We can see if we get more reports about bits of UI being
> blurry.

OK. If it comes up then it could be added to
aura::Window::OnWindowBoundsChanged().

lgtm

[jamesr, 2014-06-03 20:54:19]

The CQ bit was checked by jamesr@chromium.org

[jamesr, 2014-06-03 20:55:00]

On 2014/06/03 20:49:50, Ben Goodger (Google) wrote:
> On 2014/06/03 17:13:01, jamesr wrote:
> > On 2014/06/03 06:21:35, Ben Goodger (Google) wrote:
> > > Do you know if this issue generally affects all aura windows, or is it
just
> > the
> > > content area?
> > 
> > It potentially impacts all aura windows, but I'm not sure of another case
> where
> > it would matter.  We can see if we get more reports about bits of UI being
> > blurry.
> 
> OK. If it comes up then it could be added to
> aura::Window::OnWindowBoundsChanged().

Yes, although I'm not sure what sort of policy would make sense to apply for all
aura::Windows - it's doubtful that translate+clipping is right all of the time. 
We can figure that out when we have another use case I suppose.

[commit-bot: I haz the power, 2014-06-03 20:55:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jamesr@chromium.org/304973003/20001

[chromium-reviews, 2014-06-03 21:17:24]

On Tue, Jun 3, 2014 at 7:18 PM, <jamesr@chromium.org> wrote:

> On 2014/06/03 10:15:43, danakj (OOO_back_june_6) wrote:
>
>> Could we do something more general in the compositor where we know the
>> screen
>> space position of each quad, and could shift the uvrect to compensate for
>> the
>> positions?
>>
>
> No, because the compositor doesn't have enough context about what the
> application is trying to do for a given layer/quad.  The decision to
> translate+clip the content out has to be made at a higher level that's
> aware of
> the context (such as here where we know that we're shifting web contents
> around
> inside a frame).  For an arbitrary DelegatedRendererLayer, the compositor
> has no
> idea if the contents should be snapped+clipped or filtered or something
> else -
> consider the case where the delegated content is from an iframe with a CSS
> animation or scale being applied.
>

We could do it just in the case where all ancestors are
IdentityOrIntegerTranslation position/transform, and mark that on the quad
that we should snap it in that case?

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2014-06-03 21:31:38]

On Tue, Jun 3, 2014 at 2:17 PM, Dana Jansens <danakj@google.com> wrote:

> On Tue, Jun 3, 2014 at 7:18 PM, <jamesr@chromium.org> wrote:
>
>> On 2014/06/03 10:15:43, danakj (OOO_back_june_6) wrote:
>>
>>> Could we do something more general in the compositor where we know the
>>> screen
>>> space position of each quad, and could shift the uvrect to compensate
>>> for the
>>> positions?
>>>
>>
>> No, because the compositor doesn't have enough context about what the
>> application is trying to do for a given layer/quad.  The decision to
>> translate+clip the content out has to be made at a higher level that's
>> aware of
>> the context (such as here where we know that we're shifting web contents
>> around
>> inside a frame).  For an arbitrary DelegatedRendererLayer, the compositor
>> has no
>> idea if the contents should be snapped+clipped or filtered or something
>> else -
>> consider the case where the delegated content is from an iframe with a CSS
>> animation or scale being applied.
>>
>
> We could do it just in the case where all ancestors are
> IdentityOrIntegerTranslation position/transform, and mark that on the quad
> that we should snap it in that case?
>

But they aren't in this case - an ancestor has a transform with a
fractional translation (after considering device scale) in this case which
we're "undoing" by applying another transform to the DRL.  We'd need some
way to know that this transform is safe to undo, which seems like the
tricky part.

- James

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2014-06-03 21:38:46]

On Tue, Jun 3, 2014 at 11:31 PM, James Robinson <jamesr@google.com> wrote:

> On Tue, Jun 3, 2014 at 2:17 PM, Dana Jansens <danakj@google.com> wrote:
>
>> On Tue, Jun 3, 2014 at 7:18 PM, <jamesr@chromium.org> wrote:
>>
>>> On 2014/06/03 10:15:43, danakj (OOO_back_june_6) wrote:
>>>
>>>> Could we do something more general in the compositor where we know the
>>>> screen
>>>> space position of each quad, and could shift the uvrect to compensate
>>>> for the
>>>> positions?
>>>>
>>>
>>> No, because the compositor doesn't have enough context about what the
>>> application is trying to do for a given layer/quad.  The decision to
>>> translate+clip the content out has to be made at a higher level that's
>>> aware of
>>> the context (such as here where we know that we're shifting web contents
>>> around
>>> inside a frame).  For an arbitrary DelegatedRendererLayer, the
>>> compositor has no
>>> idea if the contents should be snapped+clipped or filtered or something
>>> else -
>>> consider the case where the delegated content is from an iframe with a
>>> CSS
>>> animation or scale being applied.
>>>
>>
>> We could do it just in the case where all ancestors are
>> IdentityOrIntegerTranslation position/transform, and mark that on the quad
>> that we should snap it in that case?
>>
>
> But they aren't in this case - an ancestor has a transform with a
> fractional translation (after considering device scale) in this case which
> we're "undoing" by applying another transform to the DRL.  We'd need some
> way to know that this transform is safe to undo, which seems like the
> tricky part.
>

Inside the renderer compositor, they are all integer/identity? Are you
referring to the position of the DRL layer itself, or an ancestor of it?
When you say non-integer, do you mean in layer space or screen space? I was
thinking in DIP/layer space, which would be a hint that it meant for it to
be pixel-aligned.


>
> - James
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2014-06-03 21:44:05]

On Tue, Jun 3, 2014 at 2:38 PM, Dana Jansens <danakj@google.com> wrote:

> On Tue, Jun 3, 2014 at 11:31 PM, James Robinson <jamesr@google.com> wrote:
>
>> On Tue, Jun 3, 2014 at 2:17 PM, Dana Jansens <danakj@google.com> wrote:
>>
>>> On Tue, Jun 3, 2014 at 7:18 PM, <jamesr@chromium.org> wrote:
>>>
>>>> On 2014/06/03 10:15:43, danakj (OOO_back_june_6) wrote:
>>>>
>>>>> Could we do something more general in the compositor where we know the
>>>>> screen
>>>>> space position of each quad, and could shift the uvrect to compensate
>>>>> for the
>>>>> positions?
>>>>>
>>>>
>>>> No, because the compositor doesn't have enough context about what the
>>>> application is trying to do for a given layer/quad.  The decision to
>>>> translate+clip the content out has to be made at a higher level that's
>>>> aware of
>>>> the context (such as here where we know that we're shifting web
>>>> contents around
>>>> inside a frame).  For an arbitrary DelegatedRendererLayer, the
>>>> compositor has no
>>>> idea if the contents should be snapped+clipped or filtered or something
>>>> else -
>>>> consider the case where the delegated content is from an iframe with a
>>>> CSS
>>>> animation or scale being applied.
>>>>
>>>
>>> We could do it just in the case where all ancestors are
>>> IdentityOrIntegerTranslation position/transform, and mark that on the quad
>>> that we should snap it in that case?
>>>
>>
>> But they aren't in this case - an ancestor has a transform with a
>> fractional translation (after considering device scale) in this case which
>> we're "undoing" by applying another transform to the DRL.  We'd need some
>> way to know that this transform is safe to undo, which seems like the
>> tricky part.
>>
>
> Inside the renderer compositor, they are all integer/identity?
>

Inside the renderer compositor there could be arbitrary transforms.  I'm
not sure how that relates to this issue, though.



> Are you referring to the position of the DRL layer itself, or an ancestor
> of it? When you say non-integer, do you mean in layer space or screen
> space? I was thinking in DIP/layer space, which would be a hint that it
> meant for it to be pixel-aligned.
>

I don't think having the offset be integer or not in DIP space is a good
hint for pixel-alignment at all.  If a device-scale-aware client was
actually trying to make things be pixel-aligned with a non-integer device
scale then they would have to specify non-integer positions in order for
things to line up correctly.

More generally, we shouldn't be trying to infer things based on "hints" in
the compositor at all.  The user(s) of the compositor (in this case a
combination of views, aura, and content) are the systems that know what
sort of visual effect they are trying to get (in this case, put web
contents inside a web frame without it being all blurry) and they should be
making all of the decisions regarding positioning and how to handle corner
cases like this.  The compositor should just do what it is told.

- James


>
>>
>> - James
>>
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2014-06-03 21:57:10]

On Tue, Jun 3, 2014 at 11:44 PM, James Robinson <jamesr@google.com> wrote:

>
>
>
> On Tue, Jun 3, 2014 at 2:38 PM, Dana Jansens <danakj@google.com> wrote:
>
>> On Tue, Jun 3, 2014 at 11:31 PM, James Robinson <jamesr@google.com>
>> wrote:
>>
>>> On Tue, Jun 3, 2014 at 2:17 PM, Dana Jansens <danakj@google.com> wrote:
>>>
>>>> On Tue, Jun 3, 2014 at 7:18 PM, <jamesr@chromium.org> wrote:
>>>>
>>>>> On 2014/06/03 10:15:43, danakj (OOO_back_june_6) wrote:
>>>>>
>>>>>> Could we do something more general in the compositor where we know
>>>>>> the screen
>>>>>> space position of each quad, and could shift the uvrect to compensate
>>>>>> for the
>>>>>> positions?
>>>>>>
>>>>>
>>>>> No, because the compositor doesn't have enough context about what the
>>>>> application is trying to do for a given layer/quad.  The decision to
>>>>> translate+clip the content out has to be made at a higher level that's
>>>>> aware of
>>>>> the context (such as here where we know that we're shifting web
>>>>> contents around
>>>>> inside a frame).  For an arbitrary DelegatedRendererLayer, the
>>>>> compositor has no
>>>>> idea if the contents should be snapped+clipped or filtered or
>>>>> something else -
>>>>> consider the case where the delegated content is from an iframe with a
>>>>> CSS
>>>>> animation or scale being applied.
>>>>>
>>>>
>>>> We could do it just in the case where all ancestors are
>>>> IdentityOrIntegerTranslation position/transform, and mark that on the quad
>>>> that we should snap it in that case?
>>>>
>>>
>>> But they aren't in this case - an ancestor has a transform with a
>>> fractional translation (after considering device scale) in this case which
>>> we're "undoing" by applying another transform to the DRL.  We'd need some
>>> way to know that this transform is safe to undo, which seems like the
>>> tricky part.
>>>
>>
>> Inside the renderer compositor, they are all integer/identity?
>>
>
> Inside the renderer compositor there could be arbitrary transforms.  I'm
> not sure how that relates to this issue, though.
>
>
>
>> Are you referring to the position of the DRL layer itself, or an ancestor
>> of it? When you say non-integer, do you mean in layer space or screen
>> space? I was thinking in DIP/layer space, which would be a hint that it
>> meant for it to be pixel-aligned.
>>
>
> I don't think having the offset be integer or not in DIP space is a good
> hint for pixel-alignment at all.  If a device-scale-aware client was
> actually trying to make things be pixel-aligned with a non-integer device
> scale then they would have to specify non-integer positions in order for
> things to line up correctly.
>

This is true, but we hide the DSF inside the compositor so that the UI can
be DIP and not have to think about it. Making it have to think about
positioning things based on the DSF to avoid blurriness would be awkward.
If it positions something at 4.4px because of DSF, we have to pick
something to go in the 4th pixel, and if it's going to be this layer's
content, unless it's really trying to position it with a sub pixel
translate (like during an animation), it seems like it would make the most
sense to position the texture content 1:1 with the pixel if we can. Why
would we want to sample outside the texture in this case, and sample
multiple pixels into each screen pixel in this case?


> More generally, we shouldn't be trying to infer things based on "hints" in
> the compositor at all.  The user(s) of the compositor (in this case a
> combination of views, aura, and content) are the systems that know what
> sort of visual effect they are trying to get (in this case, put web
> contents inside a web frame without it being all blurry) and they should be
> making all of the decisions regarding positioning and how to handle corner
> cases like this.  The compositor should just do what it is told.
>

I agree, they know what they are trying to do, but if they are just trying
to place a layer at 5,5 or whatnot, I don't think we want to make them have
to multiply by DSF, round, and divide the DSF back out. What benefit do we
get by making the UI have to think about the DSF while laying out things in
DIP?


>
> - James
>
>
>>
>>>
>>> - James
>>>
>>>
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2014-06-03 22:13:24]

On Tue, Jun 3, 2014 at 2:56 PM, Dana Jansens <danakj@google.com> wrote:

> On Tue, Jun 3, 2014 at 11:44 PM, James Robinson <jamesr@google.com> wrote:
>
>>
>>
>>
>> On Tue, Jun 3, 2014 at 2:38 PM, Dana Jansens <danakj@google.com> wrote:
>>
>>> On Tue, Jun 3, 2014 at 11:31 PM, James Robinson <jamesr@google.com>
>>> wrote:
>>>
>>>> On Tue, Jun 3, 2014 at 2:17 PM, Dana Jansens <danakj@google.com> wrote:
>>>>
>>>>> On Tue, Jun 3, 2014 at 7:18 PM, <jamesr@chromium.org> wrote:
>>>>>
>>>>>> On 2014/06/03 10:15:43, danakj (OOO_back_june_6) wrote:
>>>>>>
>>>>>>> Could we do something more general in the compositor where we know
>>>>>>> the screen
>>>>>>> space position of each quad, and could shift the uvrect to
>>>>>>> compensate for the
>>>>>>> positions?
>>>>>>>
>>>>>>
>>>>>> No, because the compositor doesn't have enough context about what the
>>>>>> application is trying to do for a given layer/quad.  The decision to
>>>>>> translate+clip the content out has to be made at a higher level
>>>>>> that's aware of
>>>>>> the context (such as here where we know that we're shifting web
>>>>>> contents around
>>>>>> inside a frame).  For an arbitrary DelegatedRendererLayer, the
>>>>>> compositor has no
>>>>>> idea if the contents should be snapped+clipped or filtered or
>>>>>> something else -
>>>>>> consider the case where the delegated content is from an iframe with
>>>>>> a CSS
>>>>>> animation or scale being applied.
>>>>>>
>>>>>
>>>>> We could do it just in the case where all ancestors are
>>>>> IdentityOrIntegerTranslation position/transform, and mark that on the
quad
>>>>> that we should snap it in that case?
>>>>>
>>>>
>>>> But they aren't in this case - an ancestor has a transform with a
>>>> fractional translation (after considering device scale) in this case which
>>>> we're "undoing" by applying another transform to the DRL.  We'd need some
>>>> way to know that this transform is safe to undo, which seems like the
>>>> tricky part.
>>>>
>>>
>>> Inside the renderer compositor, they are all integer/identity?
>>>
>>
>> Inside the renderer compositor there could be arbitrary transforms.  I'm
>> not sure how that relates to this issue, though.
>>
>>
>>
>>> Are you referring to the position of the DRL layer itself, or an
>>> ancestor of it? When you say non-integer, do you mean in layer space or
>>> screen space? I was thinking in DIP/layer space, which would be a hint that
>>> it meant for it to be pixel-aligned.
>>>
>>
>> I don't think having the offset be integer or not in DIP space is a good
>> hint for pixel-alignment at all.  If a device-scale-aware client was
>> actually trying to make things be pixel-aligned with a non-integer device
>> scale then they would have to specify non-integer positions in order for
>> things to line up correctly.
>>
>
> This is true, but we hide the DSF inside the compositor so that the UI can
> be DIP and not have to think about it. Making it have to think about
> positioning things based on the DSF to avoid blurriness would be awkward.
> If it positions something at 4.4px because of DSF, we have to pick
> something to go in the 4th pixel, and if it's going to be this layer's
> content, unless it's really trying to position it with a sub pixel
> translate (like during an animation), it seems like it would make the most
> sense to position the texture content 1:1 with the pixel if we can. Why
> would we want to sample outside the texture in this case, and sample
> multiple pixels into each screen pixel in this case?
>

It's not possible for the UI to only think in DIPs and completely ignore
physical pixels unless it wants to look blurry, which we don't want.  To
map correctly to physical pixels requires breaking some other part of the
layout.  It's a convenient fantasy most of the time to think that the DSF
can be hidden away from the UI code but sometimes that abstraction fails.

Making decisions about which pixel to choose in the compositor in cases
like this is a losing battle - the compositor just can't know what the
caller wanted.


>
>
>> More generally, we shouldn't be trying to infer things based on "hints"
>> in the compositor at all.  The user(s) of the compositor (in this case a
>> combination of views, aura, and content) are the systems that know what
>> sort of visual effect they are trying to get (in this case, put web
>> contents inside a web frame without it being all blurry) and they should be
>> making all of the decisions regarding positioning and how to handle corner
>> cases like this.  The compositor should just do what it is told.
>>
>
> I agree, they know what they are trying to do, but if they are just trying
> to place a layer at 5,5 or whatnot, I don't think we want to make them have
> to multiply by DSF, round, and divide the DSF back out. What benefit do we
> get by making the UI have to think about the DSF while laying out things in
> DIP?
>

The thing won't fit exactly no matter what you do.  You have to either clip
something or create gaps.  The decision of which to do is a UI decision, so
it has to be made by the UI layer.  In some cases we want gaps, in some we
want clip.  In this case, we're deciding to potentially create a gap on the
left/top edge of the web content area (by up to 1 phys pixel) and clip out
the bottom/right edge of the web content (again by up to 1 physical pixel).
 The reason we can make that decision in this part of the UI is because we
know based on the way that the window frame is rendered is that this
tradeoff is reasonable to make.  In other situations, the desired UI effect
might be to repeat pixels near the edge or clamp on all edges or something
else entirely.

You can't magic away device scale issues - somebody somewhere has to make a
tradeoff about how they want content to map to physical pixels.  The code
that makes those decisions has to do so with knowledge about the context
about what sort of effect it is trying to achieve.

- James

>
>
>>
>> - James
>>
>>
>>>
>>>>
>>>> - James
>>>>
>>>>
>>>
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2014-06-03 22:32:28]

On Wed, Jun 4, 2014 at 12:13 AM, James Robinson <jamesr@google.com> wrote:
>
> On Tue, Jun 3, 2014 at 2:56 PM, Dana Jansens <danakj@google.com> wrote:
>
>> On Tue, Jun 3, 2014 at 11:44 PM, James Robinson <jamesr@google.com>
>> wrote:
>>
>>> On Tue, Jun 3, 2014 at 2:38 PM, Dana Jansens <danakj@google.com> wrote:
>>>
>>>> On Tue, Jun 3, 2014 at 11:31 PM, James Robinson <jamesr@google.com>
>>>> wrote:
>>>>
>>>>> On Tue, Jun 3, 2014 at 2:17 PM, Dana Jansens <danakj@google.com>
>>>>> wrote:
>>>>>
>>>>>> On Tue, Jun 3, 2014 at 7:18 PM, <jamesr@chromium.org> wrote:
>>>>>>
>>>>>>> On 2014/06/03 10:15:43, danakj (OOO_back_june_6) wrote:
>>>>>>>
>>>>>>>> Could we do something more general in the compositor where we know
>>>>>>>> the screen
>>>>>>>> space position of each quad, and could shift the uvrect to
>>>>>>>> compensate for the
>>>>>>>> positions?
>>>>>>>>
>>>>>>>
>>>>>>> No, because the compositor doesn't have enough context about what the
>>>>>>> application is trying to do for a given layer/quad.  The decision to
>>>>>>> translate+clip the content out has to be made at a higher level
>>>>>>> that's aware of
>>>>>>> the context (such as here where we know that we're shifting web
>>>>>>> contents around
>>>>>>> inside a frame).  For an arbitrary DelegatedRendererLayer, the
>>>>>>> compositor has no
>>>>>>> idea if the contents should be snapped+clipped or filtered or
>>>>>>> something else -
>>>>>>> consider the case where the delegated content is from an iframe with
>>>>>>> a CSS
>>>>>>> animation or scale being applied.
>>>>>>>
>>>>>>
>>>>>> We could do it just in the case where all ancestors are
>>>>>> IdentityOrIntegerTranslation position/transform, and mark that on the
quad
>>>>>> that we should snap it in that case?
>>>>>>
>>>>>
>>>>> But they aren't in this case - an ancestor has a transform with a
>>>>> fractional translation (after considering device scale) in this case
which
>>>>> we're "undoing" by applying another transform to the DRL.  We'd need some
>>>>> way to know that this transform is safe to undo, which seems like the
>>>>> tricky part.
>>>>>
>>>>
>>>> Inside the renderer compositor, they are all integer/identity?
>>>>
>>>
>>> Inside the renderer compositor there could be arbitrary transforms.  I'm
>>> not sure how that relates to this issue, though.
>>>
>>>
>>>
>>>> Are you referring to the position of the DRL layer itself, or an
>>>> ancestor of it? When you say non-integer, do you mean in layer space or
>>>> screen space? I was thinking in DIP/layer space, which would be a hint
that
>>>> it meant for it to be pixel-aligned.
>>>>
>>>
>>> I don't think having the offset be integer or not in DIP space is a good
>>> hint for pixel-alignment at all.  If a device-scale-aware client was
>>> actually trying to make things be pixel-aligned with a non-integer device
>>> scale then they would have to specify non-integer positions in order for
>>> things to line up correctly.
>>>
>>
>> This is true, but we hide the DSF inside the compositor so that the UI
>> can be DIP and not have to think about it. Making it have to think about
>> positioning things based on the DSF to avoid blurriness would be awkward.
>> If it positions something at 4.4px because of DSF, we have to pick
>> something to go in the 4th pixel, and if it's going to be this layer's
>> content, unless it's really trying to position it with a sub pixel
>> translate (like during an animation), it seems like it would make the most
>> sense to position the texture content 1:1 with the pixel if we can. Why
>> would we want to sample outside the texture in this case, and sample
>> multiple pixels into each screen pixel in this case?
>>
>
> It's not possible for the UI to only think in DIPs and completely ignore
> physical pixels unless it wants to look blurry, which we don't want.  To
> map correctly to physical pixels requires breaking some other part of the
> layout.  It's a convenient fantasy most of the time to think that the DSF
> can be hidden away from the UI code but sometimes that abstraction fails.
>
> Making decisions about which pixel to choose in the compositor in cases
> like this is a losing battle - the compositor just can't know what the
> caller wanted.
>
>
>>
>>
>>> More generally, we shouldn't be trying to infer things based on "hints"
>>> in the compositor at all.  The user(s) of the compositor (in this case a
>>> combination of views, aura, and content) are the systems that know what
>>> sort of visual effect they are trying to get (in this case, put web
>>> contents inside a web frame without it being all blurry) and they should be
>>> making all of the decisions regarding positioning and how to handle corner
>>> cases like this.  The compositor should just do what it is told.
>>>
>>
>> I agree, they know what they are trying to do, but if they are just
>> trying to place a layer at 5,5 or whatnot, I don't think we want to make
>> them have to multiply by DSF, round, and divide the DSF back out. What
>> benefit do we get by making the UI have to think about the DSF while laying
>> out things in DIP?
>>
>
> The thing won't fit exactly no matter what you do.  You have to either
> clip something or create gaps.  The decision of which to do is a UI
> decision, so it has to be made by the UI layer.  In some cases we want
> gaps, in some we want clip.  In this case, we're deciding to potentially
> create a gap on the left/top edge of the web content area (by up to 1 phys
> pixel) and clip out the bottom/right edge of the web content (again by up
> to 1 physical pixel).  The reason we can make that decision in this part of
> the UI is because we know based on the way that the window frame is
> rendered is that this tradeoff is reasonable to make.  In other situations,
> the desired UI effect might be to repeat pixels near the edge or clamp on
> all edges or something else entirely.
>
> You can't magic away device scale issues - somebody somewhere has to make
> a tradeoff about how they want content to map to physical pixels.  The code
> that makes those decisions has to do so with knowledge about the context
> about what sort of effect it is trying to achieve.
>

My understanding is that in the end for each screen pixel, we're going to
pick one texture to sample from and fill the screen pixel with that, and we
use the pixel center to do that. If the layer's position is overlapping the
center, then we're going to use the layer, otherwise we'll use the DIP
pixel to the left/right of the layer, etc. So if you layout a window border
right beside a tab contents in DIP space, and that boundary ends up in the
middle of a screen pixel, we'd just end up picking the one that takes up
more of the pixel (occupies the pixel center), and on the other end of the
layer, we'd do the same. This is what we do now, except that we don't align
the pixels we choose with the screen pixels, so we end up with blurriness
if we turn on LINEAR or skipping/repeating a pixel with NEAREST. If we
align this to the screen pixels, then we avoid that. I don't think we
introduce any gaps in this way, do we?

I also don't want to introduce magic here, but I am having trouble thinking
of a case where this would do the wrong thing for us. It sounds like you
are thinking of some scenarios where this would be wrong, and the UI would
do something different with more control. Do you have a concrete example in
mind (can you draw a picture or something :)) ?

Also, do you want to have blink layout its compositor layers at sub-pixel
positions to get them screen-pixel aligned also?

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2014-06-03 23:27:47]

On Tue, Jun 3, 2014 at 3:32 PM, Dana Jansens <danakj@google.com> wrote:

> On Wed, Jun 4, 2014 at 12:13 AM, James Robinson <jamesr@google.com> wrote:My
> understanding is that in the end for each screen pixel, we're going to pick
> one texture to sample from and fill the screen pixel with that, and we use
> the pixel center to do that. If the layer's position is overlapping the
> center, then we're going to use the layer, otherwise we'll use the DIP
> pixel to the left/right of the layer, etc. So if you layout a window border
> right beside a tab contents in DIP space, and that boundary ends up in the
> middle of a screen pixel, we'd just end up picking the one that takes up
> more of the pixel (occupies the pixel center), and on the other end of the
> layer, we'd do the same.
>

Currently the window frame on the left side of the tab contents occupies an
odd number of DIP pixels.  This means with a scale of 1.5, the rightmost
edge of the tab border and the leftmost edge of the tab contents split a
physical pixel exactly down the center. Which do you consider to be "the
texture occupying the pixel center"?  Neither occupies more of the pixel,
except for floating point error, it's exactly split.



> This is what we do now, except that we don't align the pixels we choose
> with the screen pixels, so we end up with blurriness if we turn on LINEAR
> or skipping/repeating a pixel with NEAREST. If we align this to the screen
> pixels, then we avoid that. I don't think we introduce any gaps in this
> way, do we?
>

Alignment = moving something.  Assuming that the layout is tight to begin
with (i.e. does not have gaps or overlap) then any movement will either
clip something or introduce a gap somewhere.  We can't invent space out of
nowhere :)


>
> I also don't want to introduce magic here, but I am having trouble
> thinking of a case where this would do the wrong thing for us. It sounds
> like you are thinking of some scenarios where this would be wrong, and the
> UI would do something different with more control. Do you have a concrete
> example in mind (can you draw a picture or something :)) ?
>

Consider a horizontal slice across a browser window.  We have some browser
frame, then web contents, then more browser frame, then the edge of the
window.  In DIP space views decides it wants things to lay out thusly:

| 7 DIP pixels of browser frame | 600 DIP pixels of web contents | 7 pixels
of browser frame |

We tell the render process to size itself to 600 DIPs, meaning the viewport
is 600 CSS pixels wide.  We have a device scale of 1.5 so the physical
pixel size of everything (prior to this patch) would be:

| 10.5 pixels of browser frame | 900 pixels of web contents | 10.5 pixels
of browser frame |

This means the tab contents are at an offset of 10.5px from the left edge
of the window, which is why we filter and look blurry.  The tricky question
here is what should occupy the 11th and pixel from the left and right edges
of the window.  There are several possibilities depending on what we want
to optimize for.  We need to end up 921 physical pixels wide, we want to
show the web contents as accurately as possible, we don't want to have
gaps, and we want the window frame to be as symmetric as possible.

(A - snap everything to the left)
| 10 pixels of browser frame | 900 pixels of web contents | 1 pixel gap |
10 pixels of browser frame |

Here the left part of the window frame is 10 physicals and the web contents
starts at the 11th physical pixel and extends up to and including the
911th.  Then we have 11 physical pixels left in the window, but only window
frame content to put there.  We have to put a gap in somewhere (whether
it's on the left or right edge isn't important).  The way that the chrome
browser UI actually looks this means the web contents actually looks too
far left relative to the window frame in the omnibox area.

(B - snap to the right)
| 11 pixels of browser frame | 900 pixels of web contents | 10 pixels of
browser frame |

This isn't good either since the browser frame on the right is smaller than
on the left, so it won't be symmetric.

(C - shift + clip web contents)

| 11 pixels of browser frame | 899 pixels of web contents | 11 pixels of
browser frame |

This is what this patch implements.  The web contents is "shifted" by 1/2
pixel to the right, meaning that the browser frame on the left edge can
occupy a full 11 pixels and the rightmost edge of the web contents area is
clipped out by 1/3 of a DIP.  The browser frame can occupy full space on
either edge.

I would not say this is a correct *general* solution but it's a correct
solution *for this particular UI*.  I think it'd be equally mathematically
valid to say that the 11th pixel should be web contents and not browser
frame, but that'd look bad since it would mean that the browser frame would
be inconsistent with how infobars and other content below the navigation
bar render.  Kicking the web contents to the right looks better for this.

Here's what the left edge of the window looks like in Chrome Canary blown
up 8x at 1.5 scale:

http://imgur.com/rRKu2Sn

Notice how when the web contents starts the rightmost pixel of the window
frame appears to be lighter - it's actually filtering 50% of the window
frame (grey) with 50% of the leftmost web contents pixel (white).

This is with this patch:

http://imgur.com/XTsPsVw

Here the window frame fills out the 11th pixel completely and is the same
color as it is before the web contents starts.

Of course, this means that the right edge is slightly different too - but
for Chrome we want the browser frame to look the same so that means
clipping the web contents area slightly.


> Also, do you want to have blink layout its compositor layers at sub-pixel
> positions to get them screen-pixel aligned also?
>

Blink already snaps to physical (or as close as it can get with the
information it has) when painting, which means doing positioning to finer
granularity than DIPs (so subpixel in a sense).  It definitely should be
doing the same for composited layer positions to be consistent with painted
content.

- James

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-06-04 00:34:59]

Change committed as 274686

[chromium-reviews, 2014-06-04 01:42:45]

On Wed, Jun 4, 2014 at 1:27 AM, James Robinson <jamesr@google.com> wrote:

> On Tue, Jun 3, 2014 at 3:32 PM, Dana Jansens <danakj@google.com> wrote:
>
>> On Wed, Jun 4, 2014 at 12:13 AM, James Robinson <jamesr@google.com>
>> wrote:My understanding is that in the end for each screen pixel, we're
>> going to pick one texture to sample from and fill the screen pixel with
>> that, and we use the pixel center to do that. If the layer's position is
>> overlapping the center, then we're going to use the layer, otherwise we'll
>> use the DIP pixel to the left/right of the layer, etc. So if you layout a
>> window border right beside a tab contents in DIP space, and that boundary
>> ends up in the middle of a screen pixel, we'd just end up picking the one
>> that takes up more of the pixel (occupies the pixel center), and on the
>> other end of the layer, we'd do the same.
>>
>
> Currently the window frame on the left side of the tab contents occupies
> an odd number of DIP pixels.  This means with a scale of 1.5, the rightmost
> edge of the tab border and the leftmost edge of the tab contents split a
> physical pixel exactly down the center. Which do you consider to be "the
> texture occupying the pixel center"?  Neither occupies more of the pixel,
> except for floating point error, it's exactly split.
>

Note: the GL "diamond exit rules" make it that exactly one triangle (hence
quad) would own that pixel center in that case.


>
>
>> This is what we do now, except that we don't align the pixels we choose
>> with the screen pixels, so we end up with blurriness if we turn on LINEAR
>> or skipping/repeating a pixel with NEAREST. If we align this to the screen
>> pixels, then we avoid that. I don't think we introduce any gaps in this
>> way, do we?
>>
>
> Alignment = moving something.  Assuming that the layout is tight to begin
> with (i.e. does not have gaps or overlap) then any movement will either
> clip something or introduce a gap somewhere.  We can't invent space out of
> nowhere :)
>
>
>>
>> I also don't want to introduce magic here, but I am having trouble
>> thinking of a case where this would do the wrong thing for us. It sounds
>> like you are thinking of some scenarios where this would be wrong, and the
>> UI would do something different with more control. Do you have a concrete
>> example in mind (can you draw a picture or something :)) ?
>>
>
> Consider a horizontal slice across a browser window.  We have some browser
> frame, then web contents, then more browser frame, then the edge of the
> window.  In DIP space views decides it wants things to lay out thusly:
>
> | 7 DIP pixels of browser frame | 600 DIP pixels of web contents | 7
> pixels of browser frame |
>
> We tell the render process to size itself to 600 DIPs, meaning the
> viewport is 600 CSS pixels wide.  We have a device scale of 1.5 so the
> physical pixel size of everything (prior to this patch) would be:
>
> | 10.5 pixels of browser frame | 900 pixels of web contents | 10.5 pixels
> of browser frame |
>
> This means the tab contents are at an offset of 10.5px from the left edge
> of the window, which is why we filter and look blurry.  The tricky question
> here is what should occupy the 11th and pixel from the left and right edges
> of the window.  There are several possibilities depending on what we want
> to optimize for.  We need to end up 921 physical pixels wide, we want to
> show the web contents as accurately as possible, we don't want to have
> gaps, and we want the window frame to be as symmetric as possible.
>
> (A - snap everything to the left)
> | 10 pixels of browser frame | 900 pixels of web contents | 1 pixel gap |
> 10 pixels of browser frame |
>
> Here the left part of the window frame is 10 physicals and the web
> contents starts at the 11th physical pixel and extends up to and including
> the 911th.  Then we have 11 physical pixels left in the window, but only
> window frame content to put there.  We have to put a gap in somewhere
> (whether it's on the left or right edge isn't important).  The way that the
> chrome browser UI actually looks this means the web contents actually looks
> too far left relative to the window frame in the omnibox area.
>
> (B - snap to the right)
> | 11 pixels of browser frame | 900 pixels of web contents | 10 pixels of
> browser frame |
>
> This isn't good either since the browser frame on the right is smaller
> than on the left, so it won't be symmetric.
>
> (C - shift + clip web contents)
>
> | 11 pixels of browser frame | 899 pixels of web contents | 11 pixels of
> browser frame |
>
> This is what this patch implements.  The web contents is "shifted" by 1/2
> pixel to the right, meaning that the browser frame on the left edge can
> occupy a full 11 pixels and the rightmost edge of the web contents area is
> clipped out by 1/3 of a DIP.  The browser frame can occupy full space on
> either edge.
>
> I would not say this is a correct *general* solution but it's a correct
> solution *for this particular UI*.  I think it'd be equally mathematically
> valid to say that the 11th pixel should be web contents and not browser
> frame, but that'd look bad since it would mean that the browser frame would
> be inconsistent with how infobars and other content below the navigation
> bar render.  Kicking the web contents to the right looks better for this.
>
> Here's what the left edge of the window looks like in Chrome Canary blown
> up 8x at 1.5 scale:
>
> http://imgur.com/rRKu2Sn
>
> Notice how when the web contents starts the rightmost pixel of the window
> frame appears to be lighter - it's actually filtering 50% of the window
> frame (grey) with 50% of the leftmost web contents pixel (white).
>
> This is with this patch:
>
> http://imgur.com/XTsPsVw
>
> Here the window frame fills out the 11th pixel completely and is the same
> color as it is before the web contents starts.
>
> Of course, this means that the right edge is slightly different too - but
> for Chrome we want the browser frame to look the same so that means
> clipping the web contents area slightly.
>
>
>> Also, do you want to have blink layout its compositor layers at sub-pixel
>> positions to get them screen-pixel aligned also?
>>
>
> Blink already snaps to physical (or as close as it can get with the
> information it has) when painting, which means doing positioning to finer
> granularity than DIPs (so subpixel in a sense).  It definitely should be
> doing the same for composited layer positions to be consistent with painted
> content.
>
> - James
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.