Johan Rydberg wrote: > Vesa Jääskeläinen <[EMAIL PROTECTED]> writes: > >> Here is the newest modifications to video subsystem. > > Hi Vesa. What a suitable name :)
:) > Sorry for the delay, and lack of earlier comments, but I've been out > of the GRUB loop for a while. But I have a few comments on the video > subsystem; > > Correct me if I am wrong here, but what you call "render target" seems > to be what other video systems call "surface" or "drawable". Yes they are called by different names and they have a bit different semantics depending on a place they are used. > Why use the concept of a "active" render target? Why not instead let > all functions that operate on the active render target take a pointer > to a specific render target? Poking through your patch, it seems that > there a lot of the following (forgive my pseudo-code) > > grub_video_set_active_render_target (target); > // .. fill it with something .. > grub_video_fill_rect (color, 0, 0, width, height); > > grub_video_set_active_render_target (GRUB_VIDEO_RENDER_TARGET_DISPLAY); > grub_video_blit_render_target (target, 0, 0, 0, 0, width, height); > > I would feel more comfortable with the following workflow: > > grub_video_fill_rect (target, color, 0, 0, width, height); > grub_video_blit_render_target (GRUB_VIDEO_RENDER_TARGET_DISPLAY, > target, 0, 0, 0, 0, width, height); Actually that patch is already on CVS :]... so you might find it easier to read from there. But the idea here was to make it easier to make function that operates on different render targets without knowing about it. eg: set_target a call common_func set target b call common_func set target c With your proposal this would require to pass this render target pointer all around and would make video function calls longer. There is also another problem in your proposal. What happens if video driver get's changed and you still have pointers to render targets (or to raw memory)? > Also, I think it is important the user can get hold of a pointer to > the render targets data, and an exact pixel format, to do private > rendering. It would be hard to make a perfect gradient using fill_rect. That might not be possible on all architectures. That's the reason there is a bitmap support (which I am working on currently). Bitmaps are located on host memory so they can be modified. Render targets can be located on host memory or on video memory depending on arch. If they are located on video memory then there is only need to have video-video blit functionality and some archs might even provide hardware accelerated helper functions. One idea of the render targets are that they hide the real bitmap data format. So one could make a 24 bit RGB picture and then just blit it to render target and underlying video driver would convert it to correct format. This frees API user from thinking all possible formats and can concentrate to make one working implementation. If one needs to modify render target on all platforms there should be support to direct access to memory or ability to read contents of frame buffer to host memory and then write it back or requirement to store data on host memory and then do blit from there to video memory, but that is a bit costy on performance. > I'm not sure the concept of "viewports" are needed at all; instead let > the 'application' (e.g, the terminal) render into a render target, and > blit that to the screen at the desired position. To minimize memory, > the videport-render target can be a sub-render target of the main > render target (ie, they share the same buffer) unless the user wants > any fancy stuff like a background picture. Dropping support for viewports would indeed make driver code a bit simplier. It were originally designed to problems like this: Let's assume you have window on your screen. Then you have scroll bars there so the actual window contents are bigger than what is shown on screen. Now you just set viewport on target render target and it's easy to render new contents and you don't need to worry that it would be rendered out of bounds of target rectangle. And as coordinates are now related to viewport you don't need to take account that. But now that I think, one could just give screen coordinates and size and then use offset to scroll within that area. Now that I have really used the api for gfxterm and working on bitmap support I have noticed that it might be better to handle those operations on one render target and then blit the changed data to visible render target. I am not fond to your sub-render-target idea. You can always use coordinates and offsets to play with it. And the background image or color would most likely to be present. > Thanks, > Johan Thanks for your comments, Vesa Jääskeläinen _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
