Help me debug my theme port!

[gbl08ma]

Hi!
I'm the author of the port of the theme photoSkins-green for iPod Nano (original is for iPod Video by Humberto Santana). It is available for download here: http://themes.rockbox.org/index.php?themeid=1241&target=ipodnano2g

It's already the second time I'm posting that theme - the first one had some glitches and was removed. Now that I already corrected most of these glitches (they had to do with the viewports), I'm still left with another. Here's what happens:

While on the WPS, the song title as well as the album, artist and playlist position disappear and appear again some seconds later. This isn't what it should do, it should scroll the text when it's too big to fit in the viewport.
To make things even more annoying, the texts don't disappear all at once: sometimes the title is visible but artist and playlist aren't, other times the playlist and artist are visible but the title or the album aren't visible... etc.

I thought it had something to do with the timings of scrolling and switching between showing the playlist position and the next track name, but I analysed the timing and conditionals code and it is equal on the two themes (the original and my port). Obviously, the viewports are smaller and in different positions because the screen is smaller too.

And to finalize, this phenomenon happens on my real iPod Nano 2nd Gen, but not on the iPod Nano 1G simulator! This makes the bug even harder to debug. I tried with the same music files, with the same IDv3 files. Only the file format was different (on my real device I have OGG and on the simulator I have MP3). Not that the files seems to matter much, because the problem exists on the real DAP with any of my 200 music files.

If anyone with an iPod Nano (1st or 2nd Gen) could download the theme and post what they see, it'd really appreciated. If someone has an idea so solve this problem, it'd be really great. If someone has a complete solution for this, it'd be wonderful

Regards
Gabriel

[[Saint]]

gbl08ma,

Without looking at the code for your theme, this behaviour absolutely wreaks of overlapping viewports.

Opening the theme in the themeeditor will tell you very quickly if your use of viewports is less than sane.



[St.]

[gbl08ma]

I used theme editor while porting the theme and it displays correctly in it.
Now I got to go out... see you tomorrow (GMT)

[audio-i]

Without looking at the code for your theme, this behaviour absolutely wreaks of overlapping viewports.

Sometimes it can be a really long shot to give advise without looking at the code  

I got the initial version of the ported themes working on a friend's nano, and I remember that the specific problem gbl08ma is referring to, is because most or all of the viewports foreground colors (%Vf) were defined in a separate line, so correcting this should solve it.

gbl08ma, there were other problems, I don't really remember all of them, but some of them were bad placing of some things, the alternating routine that showed position in playlist and next track was wrong, that last line's text was cut at the bottom, and the bitmap strips for rewind and ffw were inaccurate, since there are three images, and the total height of the strip is 44, so it should be 45 or something divisible by 3, to avoid a weird effect . I'm attaching the version I did back then from your first upload... if you have the patience to compare it line by line to the current version you might identify all the issues I found back then, and see if they are not corrected yet in your new version. of course some things would have to be also replicated in the SBS. Hope it helps

Note to the admins: I guess .sbs and .fms didn't exist when the attachments to be allowed were defined, shouldn't they be added now?  

[[Saint]]

Sometimes it can be a really long shot to give advise without looking at the code 

From what he described, I immediately suspected overlapping viewports.

It's not just a few, but every textual viewport in the .wps that overlaps, it is quite obvious when viewed in the theme editor.

It is a classic symptom, and I have fixed countless themes for other authors to resolve this very problem.

I got the initial version of the ported themes working on a friend's nano, and I remember that the specific problem gbl08ma is referring to, is because most or all of the viewports foreground colors (%Vf) were defined in a separate line, so correcting this should solve it.

No.

There has not been a need to place %vb/f() tags in the viewport definition for quite some time now, this is by design, so that one can easily change the colour of elements and not have it irrelevantly tied to the viewport definition.

It is perfectly fine to have them where they are in the code.



[St.]

[audio-i]

There has not been a need to place %vb/f() tags in the viewport definition for quite some time now, this is by design, so that one can easily change the colour of elements and not have it irrelevantly tied to the viewport definition.

I don't know why you mention this, this has not been questioned

I got the initial version of the ported themes working on a friend's nano, and I remember that the specific problem gbl08ma is referring to, is because most or all of the viewports foreground colors (%Vf) were defined in a separate line, so correcting this should solve it.

No.
...
It is perfectly fine to have them where they are in the code.

Are you aware of problems like this one? http://forums.rockbox.org/index.php/topic,26858.0.html

I didn't say there were no overlapping viewports, I just said that the problem the OP describes, can be solved as I explained. If you are going to write an absolute "No" there, you could at least try things first (for the clarity of the OP at least)

Try changing:

Code: [Select]

# Track
%V(76,35,92,15,3)
%Vf(ffffff)
%t(6.0)%s%al%?it<%it|%fn>;%t(12.0)%al%?it<%it|%fn>

to

Code: [Select]

# Track
%V(76,35,92,15,3)%Vf(ffffff)
%t(6.0)%s%al%?it<%it|%fn>;%t(12.0)%al%?it<%it|%fn>

Edit: gbl08ma, I just noticed that when battery left is about < 14% there's no battery icon; you should either put the empty battery in the background, or add it to the battery image strip

[gbl08ma]

Thanks for all the help guys. I'm at school using my mobile to connect to the internet, and so I'll read your posts better and aply your suggestions when I get home.

In fact, on the theme editor some viewports look too big, but I left them that way with the fear that text would be cropped if I reduced the size of viewports. I will change that to see if it makes any difference.

[[Saint]]

There has not been a need to place %vb/f() tags in the viewport definition for quite some time now, this is by design, so that one can easily change the colour of elements and not have it irrelevantly tied to the viewport definition.

I don't know why you mention this, this has not been questioned

You yourself specificly stated that you believed this to be the cause of the problems in the theme...I can quote you again if you'd like...but I'm sure you can read a few posts down.

By the way...what makes you think I didn't test it?

There's no point in fixing anything else if you're going to have overlapping textual viewports with scrolling text, if the text is absolutely static then sometimes you can get away with it...but it's bad practise to do so in general.

Testing on target with sane viewports (and no other changes) results in behaviour as expected here.


EDIT: I can only debug problems that I can see, at this point the theme in question (with corrected viewports) works as expected for me.

I can't say conclusively that there is no issue with this part of the theme syntax, just as I can't say conclusively that there is no issue with any other part of the syntax...but I can say that I am not experiencing these troubles in the sim, or on device.

It is entirely possible that there is a problem here, but unless I can actually see it for myself I can't say that it exists.



[St.]

[evilnick]

You seem to be both arguing the same point, that the original theme that this was based on used a deprecated way of defining the colour.

[[Saint]]

You seem to be both arguing the same point, that the original theme that this was based on used a deprecated way of defining the colour.

Neither way is deprecated, both are perfectly acceptable syntax.



[St.]

[audio-i]

By the way...what makes you think I didn't test it?

Without looking at the code for your theme...
[St.]

Plus the fact that you're still refusing to accept that the solution I posted, works.

There's no point in fixing anything else if you're going to have overlapping textual viewports with scrolling text, if the text is absolutely static then sometimes you can get away with it...but it's bad practise to do so in general.

I agree that avoiding overlapping viewports is a good practice in general. But the solution I exposed makes it possible to display things right, even if there are overlapping viewports. That can be very useful if for some reason, e.g. to achieve a specific effect, you want to overlap viewports intentionally. Clearly from the original poster's last comment, that is not the case for him.

And as evilnick says, we agree on most of the things. Basically the theme needs a major cleanup (besides overlapping viewports, I found more issues, as I commented earlier)

[[Saint]]

I went back and tested it after you called me out on it...I assumed that was obvious...perhaps not.

I am not *refusing* to believe anything, simply passing on my findings.

I cannot verify the %vf tag not being in the viewport definition causes any problems at all...*but it does not cause any on my device.

There is a HUGE difference between refusing to believe something, and finding different results from someone else.

My tests indicated to me that simply fixing the viewports was enough to get the theme to display correctly. Your earlier testing suggests differently...but that doesn't mean I refuse to believe it.

And yes, the theme has many other problems besides this one. I am only addressing the issue of the flickering viewports.


*EDIT: replaced "as" with the correct "but" to fix a slightly misleading/confusing line.

[St.]

[audio-i]

Maybe just a couple misunderstandings on both sides caused the confusion (which is not good for the OP, that's all).

And as an example of this:

... the solution I exposed makes it possible to display things right, even if there are overlapping viewports. That can be very useful if for some reason, e.g. to achieve a specific effect, you want to overlap viewports intentionally.

there is this theme http://themes.rockbox.org/index.php?themeid=1180&target=ipodcolor; if you put %Vf in a different line, the nice text effect given doesn't work, because he uses overlapping viewports.

[DrewVosburg]

Seems like some theme testers are getting testy... =P

This is the first I've heard of themeeditor!  I'll have to look into this.  I've been making themes by hand and testing on-device for years, now I feel a bit silly not thinking to look for tools like this!

It would be nice if someone made a script to check for these types of semantic errors in the code, kind of like JSLint.  That seems like a huge undertaking, but it would be really helpful for developers.  Only problem I see is changes in syntax...

Throwing the idea out there in case someone has a lot of extra time on their hands.

[[Saint]]

there is this theme http://themes.rockbox.org/index.php?themeid=1180&target=ipodcolor; if you put %Vf in a different line, the nice text effect given doesn't work, because he uses overlapping viewports.

IIUC it should be perfectly possible to recreate the effect there without overlapping viewports.

If I get a chance later today (after sleeping, it's 6am) then I shall have a play with it.

I can't think of any reason where it would be necessary to deliberately overlap a viewport.

Seems like some theme testers are getting testy... =P

This is the first I've heard of themeeditor!  I'll have to look into this.  I've been making themes by hand and testing on-device for years, now I feel a bit silly not thinking to look for tools like this!

The theme editor was created by Robert Bieber with help from Jonathan Gordon (our skin guru) for the last Google Summer of Code (GSoC2010), at this point it is still a useful tool but it is quickly falling out of sync with the current syntax and misses many recently added tags.

It is useful, for cases like this, to be able to verify issues with viewports.

Personally, that's all I use it for...I find it quicker to code by hand still.

It would be nice if someone made a script to check for these types of semantic errors in the code, kind of like JSLint.

What types of errors are you talking about exactly?

Using %vf/b outside of a viewport definition line is not "incorrect", and any actual syntax errors will be picked up by checkwps when the author attempts to upload to the themesite.

A user can also compile checkwps themselves and run it on the theme to expose errors prior to uploading to the themesite, or just if their theme is broken and it is beyond their ability to parse the error manually.

There is also a script in the source that will update old skin syntax to new (though, it is not entirely foolproof, like many tools of it's kind).



[St.]