On Fri, Aug 14, 2009 at 3:23 PM, Vladimir 'phcoder' Serbinenko<phco...@gmail.com> wrote: > Framebuffer patch comitted and I removed framebuf branch on my repo. >> >> OK, I did not get why fbfill.h of the three private headers. >> >> Perhaps fbutil.h which already declares blit_info would be a better place >> then. >> > I haven't looked in depth how private headers were organised, I just > moved the same layout to fb*. Perhaps we need only one framebuffer > private header now >>>> 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' ? >> >> Yes, perhaps I did something wrong when reordering the calls for the first >> time. >> > It's already reordered in comitted version >>>> >>>> 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. >> >> Sorry, I just confused the >> grub_video_mode_info mode_info >> with the >> grub_vbe_mode_info_block mode_info >> >> Perhaps a more distinctive name for the latter would be helpful, though. >> > I'm ok with rename. >> This is a patch against the current (as far as git knows) framebuf branch: > This branch doesn't exist anymore since it's upstream ;) >> >> * move video_fb_fender_target structure declaration to fbutil.h > Requires me to looks at how three headers are organised. >> * remove duplicate assignment in create_render_target_from_pointer >> + quiet warning > Is already done in committed version >> * rename local variable in grub_video_vbe_setup > Good >> * remove grub_video_fb_get_video_ptr from video_fb.c (dup in fbutil.c) > Good patch too >> >> Take which you want >> > Could you send them as separate patches with changelogs? It's > especially important in your case since you have no copyright > assignment yet and we have to decide which patches can go in without > such. I'm not maintainer so I need explicit consent from them to apply > your patches before you sign copyright assignment And please consider this thread closed and post your patches in a new thread >> 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 >
-- 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
