FS#9497 : This patch adds a rotate option to the jpeg viewer.

FS#9497 - This patch adds a rotate option to the jpeg viewer.

Attached to Project: Rockbox
Opened by Gerritt Gonzales (GRaTT) - Friday, 17 October 2008, 06:26 GMT

Task Type	Patches
Category	Plugins
Status	Unconfirmed
Assigned To	No-one
Operating System	Sansa e200
Severity	Low
Priority	Normal
Reported Version	Daily build (which?)
Due in Version	Undecided
Due Date	Undecided
Percent Complete
Votes	0
Private	No

Details

This patch adds a rotate option to the jpeg viewer.
Tested on sansa e200. Works on colour targets only.
Additional work required for zoom and moving around the pict.

GRaTT

jpeg-rotate-V1.diff (9.4 KiB)

This task depends upon

Comments (13)
Related Tasks (0/0)

Comment by Gerritt Gonzales (GRaTT) - Friday, 17 October 2008, 08:39 GMT

updated
Zoom works proper now.
GRaTT

jpeg-rotate-V2.diff (21.5 KiB)

Comment by Gerritt Gonzales (GRaTT) - Sunday, 19 October 2008, 03:36 GMT

removed some changes that were not required.
some error checking for grayscale in the simulator.
black and white picts would crash the simulator.
progress bar is landscape when in landscape mode.
I am stuck when it come to scrolling around bitmaps
and true grayscale conversion.
Landscape view is not saved in settings, should it be?
GRaTT

jpeg-rotate-V3.diff (25.3 KiB)

Comment by Keith Perri (perrikwp) - Monday, 20 October 2008, 19:35 GMT

I have added a new option to this patch, auto rotate.
If it is turned on, it determines if the picture would
look best in landscape or portrait mode and automatically
rotate the image upon loading it.

This patch was tested in the Gigabeat F and iPod Video simulators only.

I also added a rotate button to the Gigabeat F button keymap because
the power button wasn't currently being used for anything in the plugin.
This feature could probably be added to other targets that have unused button combinations.

Keith

jpeg-rotate-V4.diff (26.5 KiB)

Comment by Dave Chapman (linuxstb) - Tuesday, 21 October 2008, 08:45 GMT

A few comments:

1) Please see docs/CONTRIBUTING for coding guidelines - e.g. variable names (such as SCREENHEIGHT and SCREENWIDTH) should be lower case, use spaces not TABs, follow the existing style of bracket placement (and other things, like whitespace conventions) in any files you edit.

2) Global variables should be declared "static" (such as SCREENWIDTH and SCREENHEIGHT)

3) Does this patch compile for all targets? If I'm reading correctly, the "rotate" function is only implemented for colour displays.

But in general (not related to this patch), this plugin is getting far too big for a single file. and should be split into its own directory - that would make it much easier to maintain and add features to (as well as enabling the core decoding code to be factored out for reuse elsewhere).

Comment by Gerritt Gonzales (GRaTT) - Tuesday, 21 October 2008, 15:29 GMT

There is still a lot of work to do on this patch before SVN inclusion can be considered.
Either a break through in the bitmap scrolling, I am currently stuck, or it not posible
to scroll the bitmap in landscape mode. The sansa e200 converts to grayscale ok but the
simulator would crash. There were some big differences to how the simulator handled an image
and how the sansa handled it, testing on other targets is needed.
Black and white picts would crash the sim and not be displayed correcly on the device.
@ linuxstb 1 and 2: I will look at the coding guidlines and fix in my next patch.
3)currently yes only colour targets.
Some one that really understands the conversion needs to look at this patch.
I am very rusty in my C, but I think there are probably better ways to do a rotation for all targets.
GRaTT

Comment by Keith Perri (perrikwp) - Wednesday, 22 October 2008, 04:45 GMT

Here is a resynced patch (r18858), because the jpeg code was split up recently.
This patch works on my Gigabeat F, but I wasn't sure where to stick the
rotation function in the newly split code. I stuck it in yuv2rgb.c for now,
feel free to stick it where it belongs if that is the wrong place. Thanks!

Keith

jpeg-rotate-V5.diff (82 KiB)

Comment by Keith Perri (perrikwp) - Wednesday, 22 October 2008, 04:49 GMT

sorry don't use that patch as it contains unrelated changes
Let me recreate a new one with only the jpeg changes.

Comment by Keith Perri (perrikwp) - Wednesday, 22 October 2008, 04:53 GMT

Here's the correct patch:

jpeg-rotate-V5.diff (29.2 KiB)

Comment by Keith Perri (perrikwp) - Thursday, 23 October 2008, 23:17 GMT

Re-synced to r18869.

jpeg-rotate-V5-resynced.diff (28.9 KiB)

Comment by Gerritt Gonzales (GRaTT) - Friday, 20 March 2009, 05:19 GMT

This patch has the delete option (fs#7729) as well as rotate.
GRaTT

jpeg-rotate-delete_1.0.diff (31.9 KiB)