Linux: Don't vertically tile the frame background.

Linux: Don't vertically tile the frame background.

BUG=21728
Committed: http://src.chromium.org/viewvc/chrome/26185

[tony, 2009-09-14 21:39:47]

This regressed when we got rid of the ninebox.

[Evan Stade, 2009-09-14 21:53:00]

http://codereview.chromium.org/203060/diff/1/2
File chrome/browser/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/203060/diff/1/2#newcode665
Line 665: event->area.width, surface->Height());
should this be surface->Height() - event->area.x?

[Evan Stade, 2009-09-14 21:53:18]

http://codereview.chromium.org/203060/diff/1/2
File chrome/browser/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/203060/diff/1/2#newcode665
Line 665: event->area.width, surface->Height());
On 2009/09/14 21:53:00, Evan Stade wrote:
> should this be surface->Height() - event->area.x?

s/x/y

[tony, 2009-09-14 22:00:37]

On 2009/09/14 21:53:18, Evan Stade wrote:
> http://codereview.chromium.org/203060/diff/1/2
> File chrome/browser/gtk/browser_window_gtk.cc (right):
> 
> http://codereview.chromium.org/203060/diff/1/2#newcode665
> Line 665: event->area.width, surface->Height());
> On 2009/09/14 21:53:00, Evan Stade wrote:
> > should this be surface->Height() - event->area.x?
> 
> s/x/y

I don't understand, why?  I thought the surface is just a proxy for the image.

[Evan Stade, 2009-09-14 22:19:28]

> I don't understand, why?  I thought the surface is just a proxy for the image.

if there's an expose area of x=0, y=100, width=whatever, then won't this still
tile the image, but only paint the first 100 pixels of the second iteration?

[tony, 2009-09-14 22:32:28]

On 2009/09/14 22:19:28, Evan Stade wrote:
> > I don't understand, why?  I thought the surface is just a proxy for the
image.
> 
> if there's an expose area of x=0, y=100, width=whatever, then won't this still
> tile the image, but only paint the first 100 pixels of the second iteration?

I see what you're saying.  New patch so we don't over paint.

[Evan Stade, 2009-09-14 22:39:53]

http://codereview.chromium.org/203060/diff/2002/2003
File chrome/browser/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/203060/diff/2002/2003#newcode665
Line 665: event->area.width, surface->Height() - event->area.y);
can you add a check to skip this block if surface->Height() <= event->area.y?

[tony, 2009-09-14 23:10:24]

http://codereview.chromium.org/203060/diff/2002/2003
File chrome/browser/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/203060/diff/2002/2003#newcode665
Line 665: event->area.width, surface->Height() - event->area.y);
On 2009/09/14 22:39:53, Evan Stade wrote:
> can you add a check to skip this block if surface->Height() <= event->area.y?

Done.

[Evan Stade, 2009-09-14 23:23:00]

http://codereview.chromium.org/203060/diff/1003/3
File chrome/browser/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/203060/diff/1003/3#newcode662
Line 662: if (event->area.y < surface->Height()) {
can this check go before image_name declaration?

[tony, 2009-09-14 23:38:00]

http://codereview.chromium.org/203060/diff/1003/3
File chrome/browser/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/203060/diff/1003/3#newcode662
Line 662: if (event->area.y < surface->Height()) {
On 2009/09/14 23:23:01, Evan Stade wrote:
> can this check go before image_name declaration?

No, surface depends on image_name, but I was able to move the surface->SetSource
call into the if.

[Evan Stade, 2009-09-14 23:43:48]

http://codereview.chromium.org/203060/diff/1003/3
File chrome/browser/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/203060/diff/1003/3#newcode662
Line 662: if (event->area.y < surface->Height()) {
On 2009/09/14 23:38:00, tony wrote:
> On 2009/09/14 23:23:01, Evan Stade wrote:
> > can this check go before image_name declaration?
> 
> No, surface depends on image_name, but I was able to move the
surface->SetSource
> call into the if.

sorry if I am being dense, but I don't see where image_name is used aside from
surface, so if they are both in the if block it should be ok, right?

and LGTM

[tony, 2009-09-14 23:49:56]

http://codereview.chromium.org/203060/diff/1003/3
File chrome/browser/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/203060/diff/1003/3#newcode662
Line 662: if (event->area.y < surface->Height()) {
On 2009/09/14 23:43:49, Evan Stade wrote:
> sorry if I am being dense, but I don't see where image_name is used aside from
> surface, so if they are both in the if block it should be ok, right?

We can't put |surface| in the if block because |surface| is in the if condition.

[Evan Stade, 2009-09-14 23:52:07]

http://codereview.chromium.org/203060/diff/1003/3
File chrome/browser/gtk/browser_window_gtk.cc (right):

http://codereview.chromium.org/203060/diff/1003/3#newcode662
Line 662: if (event->area.y < surface->Height()) {
(so yes, I am being dense)