Icon Reading Exception #163

Closed
RobertBColton opened this Issue Nov 4, 2014 · 9 comments

Comments

Projects
None yet
3 participants
@RobertBColton
Collaborator

RobertBColton commented Nov 4, 2014

Well this is something we don't see very often, an exception in our GMK reader. The project file in question reproduces the exception every single time it is loaded.
http://nintendocfc.com/uploads/userbooths/74/smgs_enginedemo_newcontrols.zip

Forum post:
http://enigma-dev.org/forums/index.php?topic=2334

http://pastebin.com/fmGanyhi

Gtk-Message: Failed to load module "canberra-gtk-module"
java.lang.IllegalArgumentException: Can't seek to 16,494 from 17006, position already passed (required skip: -512)
    at org.lateralgm.file.StreamDecoder.seek(StreamDecoder.java:172)
    at org.lateralgm.file.iconio.ICOFile.fillDescriptor(ICOFile.java:213)
    at org.lateralgm.file.iconio.ICOFile.fillDescriptors(ICOFile.java:198)
    at org.lateralgm.file.iconio.ICOFile.read(ICOFile.java:164)
    at org.lateralgm.file.iconio.ICOFile.<init>(ICOFile.java:129)
    at org.lateralgm.file.iconio.ICOFile.<init>(ICOFile.java:113)
    at org.lateralgm.file.GmFileReader.readSettings(GmFileReader.java:429)
    at org.lateralgm.file.GmFileReader.readProjectFile(GmFileReader.java:225)
    at org.lateralgm.file.GmFileReader.readProjectFile(GmFileReader.java:165)
    at org.lateralgm.main.FileChooser$ProjectReader.read(FileChooser.java:387)
    at org.lateralgm.main.FileChooser$1.run(FileChooser.java:564)
    at java.lang.Thread.run(Thread.java:745)

@IsmAvatar The exception occurs with lgm16b4 as well, so this is no sort of regression.
http://enigma-dev.org/docs/Wiki/LateralGM:_Revisions

@RobertBColton RobertBColton changed the title from GMK Loading Exception to Icon Reading Exception Nov 5, 2014

@RobertBColton

This comment has been minimized.

Show comment
Hide comment
@RobertBColton

RobertBColton Nov 5, 2014

Collaborator

@IsmAvatar The real issue here has nothing to do with the project flle, it is the icon the project is using. Even if I create a new project in LGM and try to use the icon there is an exception. GM8.1 and Studio seem to load this icon and attach it to the runner perfectly fine. I have uploaded the icon in question.
https://www.dropbox.com/s/unxl26xlrnexbie/Icon_1.ico?dl=0

Collaborator

RobertBColton commented Nov 5, 2014

@IsmAvatar The real issue here has nothing to do with the project flle, it is the icon the project is using. Even if I create a new project in LGM and try to use the icon there is an exception. GM8.1 and Studio seem to load this icon and attach it to the runner perfectly fine. I have uploaded the icon in question.
https://www.dropbox.com/s/unxl26xlrnexbie/Icon_1.ico?dl=0

@sorlok

This comment has been minimized.

Show comment
Hide comment
@sorlok

sorlok Nov 6, 2014

Contributor

I'll have a look; there's all sorts of edge cases with icons that could be causing this. Give me a few days; currently I'm on the road.

Contributor

sorlok commented Nov 6, 2014

I'll have a look; there's all sorts of edge cases with icons that could be causing this. Give me a few days; currently I'm on the road.

@RobertBColton

This comment has been minimized.

Show comment
Hide comment
@RobertBColton

RobertBColton Nov 6, 2014

Collaborator

@sorlok Thanks I would really appreciate it. I am not that great at reading binary but from what I can tell the basic header of the ico is correct, and I agree with you. I really hate this format because of its inconsistencies and lack of standardization.

Collaborator

RobertBColton commented Nov 6, 2014

@sorlok Thanks I would really appreciate it. I am not that great at reading binary but from what I can tell the basic header of the ico is correct, and I agree with you. I really hate this format because of its inconsistencies and lack of standardization.

@sorlok

This comment has been minimized.

Show comment
Hide comment
@sorlok

sorlok Nov 8, 2014

Contributor

Ok, I've narrowed this down slightly. Apparently, most icons consist of a 32BPP bitmap followed by a 1BPP mask. In this case, LGM reads 64x64x4 bytes of bitmap data (correct!), but then tries to read 64x64/8 bytes of mask data (incorrect!).

Note that MOST icons have the mask; I need to further narrow down how and when ICO files specify that they contain no mask data.

Contributor

sorlok commented Nov 8, 2014

Ok, I've narrowed this down slightly. Apparently, most icons consist of a 32BPP bitmap followed by a 1BPP mask. In this case, LGM reads 64x64x4 bytes of bitmap data (correct!), but then tries to read 64x64/8 bytes of mask data (incorrect!).

Note that MOST icons have the mask; I need to further narrow down how and when ICO files specify that they contain no mask data.

@RobertBColton

This comment has been minimized.

Show comment
Hide comment
@RobertBColton

RobertBColton Nov 8, 2014

Collaborator

@sorlok I don't know if it helps but...
https://en.wikipedia.org/wiki/ICO_%28file_format%29#Icon_resource_structure

Thus, in 32-bit images, the AND mask is not required, but recommended for consideration. 
Collaborator

RobertBColton commented Nov 8, 2014

@sorlok I don't know if it helps but...
https://en.wikipedia.org/wiki/ICO_%28file_format%29#Icon_resource_structure

Thus, in 32-bit images, the AND mask is not required, but recommended for consideration. 
@sorlok

This comment has been minimized.

Show comment
Hide comment
@sorlok

sorlok Nov 8, 2014

Contributor

Yep, looks like the AND mask is optional. Anyway, this fix adds a bounds check:
#164

Contributor

sorlok commented Nov 8, 2014

Yep, looks like the AND mask is optional. Anyway, this fix adds a bounds check:
#164

@RobertBColton

This comment has been minimized.

Show comment
Hide comment
@RobertBColton

RobertBColton Nov 9, 2014

Collaborator

I can confirm this issue was fully resolved by @sorlok in #164

This patch will be included in the next update on the forums and I will automatically close this ticket December 7th if no new regressions arise in the interim.

Collaborator

RobertBColton commented Nov 9, 2014

I can confirm this issue was fully resolved by @sorlok in #164

This patch will be included in the next update on the forums and I will automatically close this ticket December 7th if no new regressions arise in the interim.

@IsmAvatar

This comment has been minimized.

Show comment
Hide comment
@IsmAvatar

IsmAvatar Nov 9, 2014

Owner

@RobertBColton if the issue is fully resolved, there's no reason to leave this ticket open. If regressions are found, a new ticket can be opened for it.

Owner

IsmAvatar commented Nov 9, 2014

@RobertBColton if the issue is fully resolved, there's no reason to leave this ticket open. If regressions are found, a new ticket can be opened for it.

@RobertBColton

This comment has been minimized.

Show comment
Hide comment
@RobertBColton

RobertBColton Nov 9, 2014

Collaborator

@IsmAvatar Closing the issue as resolved. Please make sure you're satisfied with the changes in that pull.

Collaborator

RobertBColton commented Nov 9, 2014

@IsmAvatar Closing the issue as resolved. Please make sure you're satisfied with the changes in that pull.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment