On Fri, Aug 14, 2009 at 2:13 AM, Michal Suchanek<hramr...@centrum.cz> wrote: > 2009/8/13 Vladimir 'phcoder' Serbinenko <phco...@gmail.com>: >>> Considering that vbe.c and sdl.c currently aren't affected a lot >>> whether there is or there isn't encapsulation in place, I'm ok tih >>> encapsulating it but if any driver needs the breach of encapsulation >>> it will be broken and I'll post no opposition to it. >>> I'll do the encapsulation tomorrow. >> Patch attached. One incremental version (for review) and one complete >> (for patching) > > The patch as presented here works for me. > > However, I wonder some of the changes: > > In the latest round of patches struct grub_video_fbrender_target > declaration was needlessly and somewhat illogically moved from > video_fb.h to fbfill.h. fbfill.h is a private header whereas video_fb.h is public one. You're the one who requested encapsulation > > I am not sure if this is to hide the structure from other code but it > logically belongs to video_fb.h. > Again, you're the one who requested encapsulation. > In grub_video_vbe_setup function in vbe.c the default palette is > loaded before the created render target is set as active. I guess this > would be a problem if the palette was actually needed/supported. > Changing the order to create, set_active, set_palette makes the code > fail for me,though. Works for me. Have you forgotten about 'return' ? > > In this same code struct grub_video_mode_info mode_info is added in > the last round of patches but I cannot find where the changed code > touches the variable. This is somewhat odd. grep is your friend: framebuffer.mode_info.width = active_mode_info.x_resolution; and follows. Since now vbe.c can't manipulate directly the target. Again, you're the one who requested encapsulation. > > In create_render_target_from_pointer the is_allocated is set to 0 twice. Fixed > I'm running the series of tests now and when I'm finished I'll commit it. After that you're welcome to submit patches for the problems you spot (you already talked about few) > Thanks > > Michal > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel >
-- Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel