Skip to contentSkip to conversation

background_image: image centering and scaling

PATCH In review Olaf Mandel 5 participants 10 messages Aug 29, 2008 – Sep 2, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 background_image: image centering and scaling

Hello,

on (some) computers with LCD monitors, the image is not automatically scaled to full screen if the video mode is set to a lower resolution. So the user has to set the gfxmode variable to the correct value in this case. But then the background image provided by their distribution needs to be rescaled by the user to fit the new screen resolution of grub.

It would be much preferable (IMO) to have grub2 rescale an existing image automatically on loading. This seems to be the cleaner, more sustainable solution: no extra files in the file system, works nicely even if the user changes the screen resolution manually, ...

This patch adds two new functions to video/bitmap.c:
grub_video_bitmap_scale() and grub_video_bitmap_scale_keepaspect(). They are called from term/gfxterm.c after reading in the bitmap in grub_gfxterm_background_image_cmd(). At the moment, the scaling algorithm used is very simple, no interpolation is used. But this should be a good starting point to expand into smooth scaling later.

Additionally, this patch adds the ability to place the bitmap at any position of the screen, not only in the top/left corner. This is done by extending redraw_screen_rect() to also clear the areas to the left and top of the image.

These changes can be controlled at by editing term/gfxterm.c: the three preprocessor variables BITMAP_CENTERED, BITMAP_SCALED and BITMAP_KEEPASPECTRATIO default to 1, but setting them to 0 restores the old behavior.

This patch fixes one minor bug in grub_gfxterm_background_image_cmd():
the stored height of the bitmap was its width. This caused the screen to not be cleared correctly in some cases.

TODO:
* Use a better scaling algorithm (linear or bi-linear interpolation).
* Change the scaling algorithm to work in-place. At the moment, a new memory location is acquired, and the old one is freed after copying (change malloc -> realloc).

Best regards,
Olaf Mandel

Signature · 3 lines
Olaf Mandel   <[email protected]>   <http://www.olaf.mandel.name/>
PGP key:      1024D/33398848 2002-09-19
Fingerprint:  0E33 BEA6 1A71 9C5E 62BD  FC0E 99A7 D2C6 3339 8848
bitmap.c +76 -0
Index: video/bitmap.c
===================================================================
@@ -245,6 +245,82 @@
   return bitmap->data;
 }
 
+grub_err_t
+grub_video_bitmap_scale (struct grub_video_bitmap **out,
+                         struct grub_video_bitmap *in,
+                         unsigned int width, unsigned int height)
+{
+  grub_err_t res;
+  unsigned int x, y;
+  unsigned int w, h;
+
+  if (!in)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid argument.");
+  w = in->mode_info.width;
+  h = in->mode_info.height;
+
+  res = grub_video_bitmap_create (out, width, height,
+                                  in->mode_info.blit_format);
+  if (res != GRUB_ERR_NONE)
+    return res;
+
+  /* This is a very simple scaling, just copying the pixel value of the
+   * pixel to the top / left of the correct position.  */
+  for (y = 0; y < height; ++y)
+  {
+    unsigned int offsy = y * (*out)->mode_info.pitch;
+    unsigned int oldy;
+    unsigned int oldoffsy;
+
+    oldy = y * h / height;
+    oldoffsy = oldy * in->mode_info.pitch;
+
+    for (x = 0; x < width; ++x)
+    {
+      unsigned int offsx = x * in->mode_info.bytes_per_pixel;
+      unsigned int oldx;
+      unsigned int oldoffsx;
+
+      oldx = x * w / width;
+      oldoffsx = oldx * in->mode_info.bytes_per_pixel;
+
+      grub_memcpy ((char *)(*out)->data + offsy + offsx,
+                   (char *)in->data + oldoffsy + oldoffsx,
+                   in->mode_info.bytes_per_pixel);
+    }
+  }
+
+  return GRUB_ERR_NONE;
+}
+
+grub_err_t
+grub_video_bitmap_scale_keepaspect (struct grub_video_bitmap **out,
+                                    struct grub_video_bitmap *in,
+                                    unsigned int maxwidth,
+                                    unsigned int maxheight)
+{
+  unsigned int w;
+  unsigned int h;
+  unsigned int width = maxwidth;
+  unsigned int height = maxheight;
+
+  if (!in)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid argument.");
+  w = in->mode_info.width;
+  h = in->mode_info.height;
+
+  /* Is the new region wider than the old bitmap? Use products instead of
+   * fractions.  */
+  if ( w * maxheight < maxwidth * h)
+    width = w * maxheight / h;
+
+  /* Is the new region higher than the old bitmap?  */
+  if ( w * maxheight > maxwidth * h)
+    height = h * maxwidth / w;
+
+  return grub_video_bitmap_scale (out, in, width, height);
+}
+
 /* Initialize bitmap module.  */
 GRUB_MOD_INIT(video_bitmap)
 {
grub/bitmap.h +8 -0
Index: include/grub/bitmap.h
===================================================================
@@ -67,4 +67,12 @@
 
 void *grub_video_bitmap_get_data (struct grub_video_bitmap *bitmap);
 
+grub_err_t grub_video_bitmap_scale (struct grub_video_bitmap **out,
+                                    struct grub_video_bitmap *in,
+                                    unsigned int width, unsigned int height);
+
+grub_err_t grub_video_bitmap_scale_keepaspect (struct grub_video_bitmap **out,
+                                               struct grub_video_bitmap *in,
+                                               unsigned int maxwidth,
+                                               unsigned int maxheight);
 #endif /* ! GRUB_BITMAP_HEADER */
ChangeLog +10 -0
Index: ChangeLog
===================================================================
@@ -1,3 +1,13 @@
+2008-08-28  Olaf Mandel  <[email protected]>
+
+	* term/gfxterm.c: Correctly store height of background image, add
+	support for centering images on viewport (opposed to having them in
+	the upper-left corner). Use new scaling feature below.
+
+	* video/bitmap.c: Add ability to scale pictures to arbitrary
+	dimensions (very simple scaling at the moment, linear or bilinear
+	scaling would be better).
+
 2008-08-28  Robert Millan  <[email protected]>
 
 	* util/biosdisk.c (find_grub_drive): Declare missing `i' variable.
gfxterm.c +98 -14

163 lines not on this page · raw message

#1 lore raw
Quoted 34 lines · Olaf Mandel

Hello,

on (some) computers with LCD monitors, the image is not automatically scaled to full screen if the video mode is set to a lower resolution. So the user has to set the gfxmode variable to the correct value in this case. But then the background image provided by their distribution needs to be rescaled by the user to fit the new screen resolution of grub.

It would be much preferable (IMO) to have grub2 rescale an existing image automatically on loading. This seems to be the cleaner, more sustainable solution: no extra files in the file system, works nicely even if the user changes the screen resolution manually, ...

This patch adds two new functions to video/bitmap.c:
grub_video_bitmap_scale() and grub_video_bitmap_scale_keepaspect(). They are called from term/gfxterm.c after reading in the bitmap in grub_gfxterm_background_image_cmd(). At the moment, the scaling algorithm used is very simple, no interpolation is used. But this should be a good starting point to expand into smooth scaling later.

Additionally, this patch adds the ability to place the bitmap at any position of the screen, not only in the top/left corner. This is done by extending redraw_screen_rect() to also clear the areas to the left and top of the image.

These changes can be controlled at by editing term/gfxterm.c: the three preprocessor variables BITMAP_CENTERED, BITMAP_SCALED and BITMAP_KEEPASPECTRATIO default to 1, but setting them to 0 restores the old behavior.

This patch fixes one minor bug in grub_gfxterm_background_image_cmd():
the stored height of the bitmap was its width. This caused the screen to not be cleared correctly in some cases.

Quoted 5 lines

TODO:
* Use a better scaling algorithm (linear or bi-linear interpolation).
* Change the scaling algorithm to work in-place. At the moment, a new memory location is acquired, and the old one is freed after copying (change malloc -> realloc).

Hi Olaf,

Thanks for your effort, but there is already code that does this, altought not committed yet. Colin D Bennett has worked under GSoC 2008 program to create graphical menu code which includes better scaler and another enhancements also relating to graphical terminal. So unfortunately your code cannot be used as is.

You can have a sneak peak of the feature at address:
http://grub.gibibit.com/

However, please wait that we have had enough time to integrate that completely before continuing to enhance it. (Should you do that)

Next time, try to discuss it first before implementing ;)

I will however look at if there are some ideas that you have provided what is still missing from new graphical menu implementation that should be there.

Thanks,
Vesa Jääskeläinen

#2 lore raw

Am Freitag, den 29.08.2008, 17:56 +0300 schrieb Vesa Jääskeläinen:

Quoted 1 line

Next time, try to discuss it first before implementing ;)

Actually this is more my fault, because I hadn't Colin's GsoC project in mind. Shame on me :(

He send a different patch to Debian BTS, related to this and then he just came up with the idea that GRUB should resize the picture itself. So I just told him to forward this and I even saw now that Robert did it too :)

http://bugs.debian.org/495754

Signature · 1 line
Felix Zielcke
#3 lore raw
In reply to Vesa Jääskeläinen

Vesa Jääskeläinen schrieb:
-Snipp-

Quoted 3 lines

I will however look at if there are some ideas that you have provided what is still missing from new graphical menu implementation that should be there.

-Snipp-

Hello,

thanks. I must admit to not looking at the source code for the project. Will scaling also be used for "simple" background_image commands? If yes and if the scaling does preserve the aspect ratio of the image, then the background picture is not necessarily in the upper left corner of the screen (an assumption in the code in grub2 trunk).

I hope some version of the gfxmenu project finds its way into the trunk of the grub2 repository soon. :-)

Best regards,
Olaf

Signature · 3 lines
Olaf Mandel   <[email protected]>   <http://www.olaf.mandel.name/>
PGP key:      1024D/33398848 2002-09-19
Fingerprint:  0E33 BEA6 1A71 9C5E 62BD  FC0E 99A7 D2C6 3339 8848
#4 lore raw
Quoted 5 lines · Olaf Mandel

thanks. I must admit to not looking at the source code for the project. Will scaling also be used for "simple" background_image commands? If yes and if the scaling does preserve the aspect ratio of the image, then the background picture is not necessarily in the upper left corner of the screen (an assumption in the code in grub2 trunk).

Basically there is no background_image command anymore in new implementation. Idea is that you specify behavior in theme file. In theme file you could have attributes what is the scaling behavior (if it was not there already).

Quoted 2 lines

I hope some version of the gfxmenu project finds its way into the trunk of the grub2 repository soon. :-)

Me too :). It just needs to be fine tuned and then committed. We should receive sliced patches in here in near term. Colin will be on holiday for a while so don't hold your breath.

#5 lore raw
Quoted 11 lines · Vesa Jääskeläinen
Quoted 5 lines · Olaf Mandel

thanks. I must admit to not looking at the source code for the project. Will scaling also be used for "simple" background_image commands? If yes and if the scaling does preserve the aspect ratio of the image, then the background picture is not necessarily in the upper left corner of the screen (an assumption in the code in grub2 trunk).

Basically there is no background_image command anymore in new implementation. Idea is that you specify behavior in theme file. In theme file you could have attributes what is the scaling behavior (if it was not there already).

Actually I have left the 'background_image' in the gfxterm module, and the background of the terminal windows contents (but not the frame) is still controlled through it. I did add scale-to-fit functionality to it, however.

Quoted 6 lines
Quoted 2 lines

I hope some version of the gfxmenu project finds its way into the trunk of the grub2 repository soon. :-)

Me too :). It just needs to be fine tuned and then committed. We should receive sliced patches in here in near term. Colin will be on holiday for a while so don't hold your breath.

I'm working on slicing-and-dicing some patches up today! I probably won't have everything done by Wednesday (when I'm leaving) but I hope to at least have several patches ready.

Regards,
Colin

#6 lore raw
Quoted 11 lines · [email protected] wrote:

On Sat, 30 Aug 2008 18:00:27 +0300, Vesa Jääskeläinen <[email protected]>

Quoted 5 lines

Olaf Mandel wrote:
Basically there is no background_image command anymore in new implementation. Idea is that you specify behavior in theme file. In theme file you could have attributes what is the scaling behavior (if it was not there already).

Actually I have left the 'background_image' in the gfxterm module, and the background of the terminal windows contents (but not the frame) is still controlled through it. I did add scale-to-fit functionality to it, however.

Ok. Then we will change it at later date then. Other new functionality has higher priority :)

Quoted 9 lines
Quoted 5 lines
Quoted 2 lines

I hope some version of the gfxmenu project finds its way into the trunk of the grub2 repository soon. :-)

Me too :). It just needs to be fine tuned and then committed. We should receive sliced patches in here in near term. Colin will be on holiday for a while so don't hold your breath.

I'm working on slicing-and-dicing some patches up today! I probably won't have everything done by Wednesday (when I'm leaving) but I hope to at least have several patches ready.

Nice. Waiting for them :)

Thanks,
Vesa Jääskeläinen

#7 lore raw
In reply to Olaf Mandel
Quoted 16 lines · On Fri, 29 Aug 2008 22:11:02 -0700 Olaf Mandel <[email protected]> wrote:

Vesa Jääskeläinen schrieb:
-Snipp-

Quoted 3 lines

I will however look at if there are some ideas that you have provided what is still missing from new graphical menu implementation that should be there.

-Snipp-

Hello,

thanks. I must admit to not looking at the source code for the project. Will scaling also be used for "simple" background_image commands? If yes and if the scaling does preserve the aspect ratio of the image, then the background picture is not necessarily in the upper left corner of the screen (an assumption in the code in grub2 trunk).

Quoted 5 lines

I hope some version of the gfxmenu project finds its way into the trunk of the grub2 repository soon. :-)

Best regards,
Olaf

Hi Olaf,

Thanks for your work! It would be great if you wanted to enhance my code by adding a preserve-aspect-ratio mode; currently my background_image command scales to fit the screen without regard for the image's original aspect ratio. Currently the '--mode' / '-m' option to 'background_image' accepts the following values:

stretch
Stretch the image to fit without regard for the original aspect ratio.

normal
Do not scale the image; origin anchored to upper-left corner.

I think if you added a "fit" option which would fit the image preserving aspect, that would be nice. Also, you could either add a "center" option which would center the background image without altering its size. I think the "normal" option will not be very desirable when scaling and centering are both available, since I can't imagine why you'd want an image stuck in the corner instead of centered or scaled in some way.

If you want to see the code for this, take a look at the message I sent to the grub-devel list today entitled "[PATCH] GSoC #09 Bitmap scaling". This patch may require some of the prior patches as well, however (possibly #06 and/or #07).

Regards,
Colin

#8 lore raw

Colin D Bennett schrieb:
-Snipp-

Quoted 4 lines

Thanks for your work! It would be great if you wanted to enhance my code by adding a preserve-aspect-ratio mode; currently my background_image command scales to fit the screen without regard for the image's original aspect ratio. [...]

-Snipp-

Hello,

I have a patch that adds one more setting for --mode that allows the picture to be scaled while keeping the aspectratio constant: fit. Also, there is now a second option --pos / -p that has 9 possible settings:
tl, t, tr, l, c, r, bl, b, br. The center-position is the default.

Start off with SVN r1845, apply the GsoC patch 09_bitmap-scaling.patch and then the one attached here.

The patch does not work correctly for me though: the string comparisons, e.g.:

grub_strcmp (state[BACKGROUND_CMD_ARGINDEX_MODE].arg, "stretch")

do not work reliable. On some occasions, the command works as expected, but often the command goes with the default value, even if something else was specified. This appears to be random... For example, the command:

background_image --mode=normal --pos=tl /grub/grub.png

will often display the image fitted into the larger screen (wrong), and sometimes as a small picture in the upper left corner (correct).

I have had no luck trying to output the content of the variables on the screen or the like... What would the correct code for that be? I hope someone finds the bug or can tell me how to continue about debugging it myself.

Best regards,
Olaf Mandel

Signature · 3 lines
Olaf Mandel   <[email protected]>   <http://www.olaf.mandel.name/>
PGP key:      1024D/33398848 2002-09-19
Fingerprint:  0E33 BEA6 1A71 9C5E 62BD  FC0E 99A7 D2C6 3339 8848
include/grub/bitmap_scale.h +7 -0
diff -urN grub2/include/grub/bitmap_scale.h grub3/include/grub/bitmap_scale.h
@@ -51,4 +51,11 @@
                                  enum
                                  grub_video_bitmap_scale_method scale_method);
 
+grub_err_t
+grub_video_bitmap_create_fitted (struct grub_video_bitmap **dst,
+                                 int max_width, int max_height,
+                                 struct grub_video_bitmap *src,
+                                 enum
+                                 grub_video_bitmap_scale_method scale_method);
+
 #endif /* ! GRUB_BITMAP_SCALE_HEADER */
term/gfxterm.c +184 -38

279 lines not on this page · raw message

video/bitmap_scale.c +46 -0

49 lines not on this page · raw message

#9 lore raw
In reply to Vesa Jääskeläinen
Quoted 3 lines · Vesa Jääskeläinen

Thanks for your effort, but there is already code that does this, altought not committed yet.

My apologises; I was unaware of this when I told Olaf to go ahead and bring this to grub-devel. Olaf, sorry if I wasted your time, I should have known better.

Signature · 5 lines
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."