palette indexes in eft-theme.c should not be prefixed with 0x

[guidol]

Binary package hint: usplash-dev

As the palette indexes in the usplash_theme struct are short integers from 0 to 255 you should NOT prefix them with 0x. This might cause confusion among readers and I don't see why it would be necessary anyway.

[Matthew Woerly (nattgew)]

Thank you for your report. I apologize that no one has gotten back to you about this yet.
Is this still the case?

[Saivann Carignan (oxmosys)]

As far as I can see, it's now fixed in usplash and usplash-theme-ubuntu sources. Setting status to fix released.

[Saivann Carignan (oxmosys)]

I was wrong, just looked with my good eyes, the problem still exist. However, setting Importance to low since this is only a example theme which is not used.

[Colin Watson (cjwatson)]

The attached branch is surely wrong. For example, it changes 0x156 to 156. Those are different numbers! (As such, the statement in the bug report that they are short integers from 0 to 255 is also incorrect.) Did you test this change? Also, please use UNRELEASED in the changelog rather than a distribution name when you aren't the one who's going to be making the upload, to reduce confusion.

It's probably OK in principle to make this change since it brings usplash's example theme into line with e.g. usplash-theme-ubuntu.

[Saivann Carignan (oxmosys)]

Colin Watson : Thanks for your comment. In your opinion, is it worth to keep the edgy theme in usplash sources when the ubuntu default theme is already included? If yes, Is it possible for you to tell me how to translate 0x* values to the right ones so I can create a correct branch for intrepid? My old (deleted) branch did not use the same colors but was working correctly.

[Colin Watson (cjwatson)]

I think it's worth having an example theme distributed with usplash itself, yes.

0x means that what follows is a hexadecimal number, i.e. in base 16. Decent calculators should be able to do base conversion for you.

[Saivann Carignan (oxmosys)]

Colin Watson : Thank your for these informations. I converted hexadecimal numbers using PHP hexdec function, and verified the values with the calculator from http://www.mrcalculator.com/hexdec.html . I built the patched example theme and tested it, the colors seems perfect (yellow and brown). The current un-patched eft theme shows different colors at each terminal lines, this is probably the cause of the hexadecimal values.

The new branch is attached to the bug report, ready for your review.

[red_team316 (redteam316)]

Saïvann Carignan:
I looked at your "fix" for the palette colors. While the original code was misleading by using 0x156, changing it to 342 is also incorrect. An 8-bit graphic by definition can only have a max of 256 colors. I know, the 0x156 displays red for the Failure status, but where the heck does that color even come from??? The real fix is getting the pallete indexes to actually work correctly at all because palette 156 is not red and the example images only have 255 colors in them. I have tried several different images and can tell you that the color you will see does not match the palette index color.

Is there another bug for this that I'm missing as it should not matter wither you use 0x0 thru 0xff(hex), or 0 thru 255(dec)

[Saivann Carignan (oxmosys)]

red_team316 : Thanks for your comment. You probably know this better than I do. I only know that hexadecimal values result in weird random colors and that converting the values to decimal fixed the random colors problem and looked like the "real" colors (brown and yellow). However, I have no proof that my fix use acceptable values or that we should really change hexadecimal values to decimal. I only know that current hexadecimal values seem to be wrong since they sometime result in ugly random colors.

[red_team316 (redteam316)]

Okay, I've tested a bit more and apparently what was throwing me off is the fact that "text background" does not palette properly. Try using input similar to what I've got posted below. Just don't set them all to zero(black) as for some reason that actually worked. It should give you a rectangle of color where the text box is but it doesn't! Can someone confirm this as it should be pretty quick.

/* Palette indexes */
.background = 254,
.progressbar_background = 254,
.progressbar_foreground = 254,
.text_background = 254,
.text_foreground = 254,
.text_success = 254,
.text_failure = 254,

[Saivann Carignan (oxmosys)]

red_team316 : I compiled the theme with "254" as you suggested and here's a screenshot of the result, white text on light pink

[red_team316 (redteam316)]

Saïvann Carignan:
Well, I give your fix 2 thumbs up. It works perfect for me too aside from the new bug we found.
Could you file a new bug and post the info with it. It's really noticeable especially when using 3's or some other low palette number. Typically, the first several palette indexes in an image will be the darkest colors and so close to black that it's really hard to tell the difference I.E. 0x000000 = black, 0x020202 = very close to black. But these colors show up as mildly bright greens or magenta.

[Saivann Carignan (oxmosys)]

red_team316 : I think that you would be the good person to submit the new bug because I don't really understand where we found a new bug. If you see a issue that should be fixed, I thank you in advance for your work! Don't hesitate to ask me to test bugs and patches!

[red_team316 (redteam316)]

Yea, I'll post it. I'll use the image and .c you posted as example. I dont have a digital camera. I'm guessing there is no other way to take a screenshot of a usplash...

Also, I tested the fix you submitted with the eft-theme example and I'm pretty sure those palette indexes of 342 and 369 are still wrong, when it shows the OK/Fail text, the colors keep changing...this behavior isn't intended.
I'm suggesting to keeping them with 0-255, to me it's just a no-brainer, but to others it may not be.

The first 2 OK's appear pink.
2nd OK red
3rd OK yellow
4th OK orange
5th OK green
Failed green
6th OK green and turns pink near the end of the test

In theory, all of the OK statuses should display as one color, but as I've posted above, they dont. I'm guessing it is pointing to memory or framebuffer thats changing and no error handling to prevent it.

attached is a modified eft-theme and my bash script I use to test with:
The only mod I did is position the text box correctly on the 800x600 res. It's pretty bad when the example usplash can't even position the text box in the right spot :/ In my opinion, the Example theme should be updated and I would consider it very important as anyone wanting to make their own is going to start there.

My suggestion is to change your 'text success' from 369 to 49 to match 'text foreground', and to change 'text failure' to '44', the 'progress foreground' to 156.

As I stated above, I think this fix needs to be merged, but not before the palette indexes are corrected so that they aren't higher than the number of colors.

To Colin: I haven't looked at the actual usplash code that these values point to but there shouldn't be any reason why anything larger than a short integer is needed since we are confined to 256 colors.

[Saivann Carignan (oxmosys)]

red_team316 : I also think that your fix is the right one. I found that using values higher than 255 causes display problems with some usplash resolutions.

Concerning the multicolors problem, I found that the colors look correct during boot until "Checking root file system...". Then the colors are different at each lines (attached screenshots).

You can test and screenshot usplash screens by booting the ubuntu into VirtualBox. It's fast and useful ;-) .

[red_team316 (redteam316)]

Yep, those pics agree with what I posted above.

Thanks for the Virtualbox tip. I wouldn't say it's fast as I'd have to rebuild an iso just for a screenie, but VERY useful. Thats thinking out of(or in this case in the) box :)

About the progress foreground being 156, if I remember correctly, its a tan/beige color in the palette, maybe find a color in the palette that is more orangish such as 100, since the imagebar is orangish. I couldn't really test those colors, as I'm not sure how since the progress bar drawing functions use images. Would you just set the throbber images to NULL to get a plain-jane progress bar?

Well, consider it your fix Saïvann, as I would have still been scratching my head had I not found this page, I probably would have kept using hex colors lol. Please update your branch or whatever is needed, as my focus in the usplash world is on making usplashmagick (a standalone usplash creator that will tie into reconstructor 3).

Hopefully the fix will be included by intrepid.

[Saivann Carignan (oxmosys)]

red_team316 : Thanks for your guidance and help on this! My knowledge and time to fix this is not sufficient here so I can only let the branch here as it is and change the status to "in progress" rather than "fix available". If at anytime I can do something that doesn't require developer skills, don't hesitate to tell me.

[red_team316 (redteam316)]

Well, looking at the related branches at the top of the page, it appears that it is your branch. I cannot change it but it would appear that you can. Are you saying it won't let you change your own branch? I would think all you would need to do is make the fix on your computer and then do a bzr commit to update it(I'm more familiar with svn than bzr but if I remember correctly, it's commit command is almost identical)
Branch page:
Update this branch: You cannot upload to this branch. Only Saïvann Carignan can upload to this branch.

I understand that someone else will have to actually merge it with the main usplash code. I was thinking you could update your branch to rev 228 to the corrected palette colors, and I would think that then the status should be "Best Fix Available" as the solution is pretty obvious and the actual person merging can read our proofs posted above.

As far as this page, once your branch is updated, wouldn't the status need to be changed to "Fix commited"?

[Saivann Carignan (oxmosys)]

red_team316 : Yes I can update the branch, I thought that you said that your previous fix was still not correct and that we should keep hexadecimal values. Do you want me to push your eft-theme.c (http://launchpadlibrarian.net/16719295/eft-theme.c) to that branch? If you think that this is a acceptable fix, then I'll go for it, or you can also publish your own branch. I just don't have necessary skill to know if this fix is really acceptable. Also, note that the problem with lines of different colors also appears with your fix (might be another bug, as you said).

[red_team316 (redteam316)]

Make these changes below to your branch in the 3 places they apply:

 /* Palette indexes */
 .background = 0,
   .progressbar_background = 7,
   .progressbar_foreground = 100,
 .text_background = 21,
 .text_foreground = 49,
 .text_success = 49,
 .text_failure = 44,

With these values, you should not experience the colors randomly changing. If you do for some reason, then let me know.
I personally think that this is the best fix possible.

[Saivann Carignan (oxmosys)]

red_team316 : Thanks a lot for your guidance. You're right. It's works correctly when using only values lower than 255. I updated my branch to contains your fix.

[Saivann Carignan (oxmosys)]

Colin Watson : Can you take a look at the updated branch that contains red_team316 works?

[Saivann Carignan (oxmosys)]

Because the bug report (and how suggested a the fix) is not subscribed to the bug report anymore, that usplash is about to be dropped by lucid and that I don't have required knowledge to do more with the current branch, I'm marking this bug as invalid and removing my branch.