Re: [Nautilus-list] Desktop background, fixes and hacks

[Darin Adler <darin bentspoon com>]

On Monday, July 23, 2001, at 10:58  PM, Owen Taylor wrote:

> The two fixes I think should definitely go in the mainline.  I'm less
> sure about the hack. It reduces nautilus's share of CPU usage on my
> (fast) machine from ~10% to ~1% when moving a window over a otherwise
> empty desktop background, so it's a pretty big effect, but the
> need to change GnomeCanvas makes things ugly.

I'd like to see all three changes rolled into CVS. The result for the user 
is more important than a bit of code ugliness, in my opinion. Reducing 
complaints about slowness is the #1 priority for Nautilus right now.

Here are some thoughts:

    1) I'd prefer to see the turn_on_bg_hack() code entirely in eel rather 
than in Nautilus. One way to do this might be to add an eel call that does 
the gdk_window_set_back_pixmap() instead of using the current idiom of 
doing eel_background_get_desktop_background_window() and calling 
gdk_window_set_back_pixmap(). Then, eel could do the right thing depending 
on whether the canvas hack is there or not (making a repeated copy of the 
passed in pixmap if necessary, I guess).

    2) I have little influence about new releases of gnome-libs, but I 
think it would be nice to get this hack in there and see if we can get a 
new release, since the Nautilus speedup is noticeable.

    3) I'd love it if you could get the ball rolling so that GnomeCanvas 
for Gnome 2.0 has appropriate hooks (if you prefer, the better ones you 
suggest rather than what you used here) so that we don't have to break the 
API freeze to get this right for Gnome 2.0.

    4) Do you know how portable the use of gmodule is here? I ask because 
when I used gmodule to find FAM, it turned out to be a portability problem.
 I don't remember exactly why, but since it had to do with file names, I 
guess it probably is not an issue here.

    5) I don't quite get why you use the gmodule chicanery to turn on the 
bg hack, but object data with a pre-arranged name to check if the bg hack 
is on. If the gmodule stuff was all in eel, not Nautilus (see #1 above), 
and you added a get call as well as a set call, then you could have the 
object data be entirely private to gnome-libs, which is enough cleaner 
that I think it's worth it.

    6) I personally would use a name which emphasizes the "will use 
clear_area to draw background if no objects are there" aspect of the hack,
 to be clear what the flag means, rather than emphasizing the "hack" 
aspect of it. Putting the emphasis on "hack"  is clear on why the code is 
there, admirably modest, and could even help deflect criticism, but doesn'
t help people understand what the code does.

    7) You should call it the canvas_bg_hack instead of the 
nautilus_bg_hack, I think. But hey, maybe not.

    8) I'd like to see a comment in the code that explains the 128 (as you 
explained it in the email), and perhaps even a #define.

    9) Last, but least, the code you added to Nautilus and Eel doesn't 
follow the Nautilus coding style. I'd be amenable to changing the style 
guide, since the set of people working on Nautilus has changed, but if we 
don't , we should change the patch before applying it. Two things I notice 
is that the patch has code that declares variables in local blocks 
(something I do in my own code, but the coding style forbids for fairly 
good reason) and uses "if (x)" rather than "if (x != NULL)".

As long as you consider these points, I'd be happy to see these changes 
committed to Nautilus and Eel. My approval doesn't mean much for 
gnome-libs, of course.

    -- Darin