Added native android tiling support

[FabricioCabeca]

Border now uses the implementation tiling support
Default drawTile now deals with its own limits, no need to clip rect outside

PS: didn't have a use case for image border with an arrow here, please, double check this before merging. Also, please double check this with iOS just to be sure, couldn't test it here. I've also noticed it fixed the sidemenu covering the shadow.

PS2: android native tiling is fast 👍

[codenameone]

Commenting here just to make sure you see this. The pull request caused a regression for some cases. See attached image:

[FabricioCabeca]

Strange, this looks like an issue I had before making sure there was a clip in the default tileImage() implementation, would you send me the code from the attached image so I can test it here ?

[FabricioCabeca]

Now that I noticed, it seems like the edge case I've mentioned in this pull request that I couldn't test here. I believe I can recreate this here so I can fix ...

[FabricioCabeca]

I couldn't reproduce this here, it worked as expected with an arrow at top or left, using default tileImage or the android optimized one. BUT, I did test the default tileImage without clipping and it did render just like the reported one, so I would guess that the arrow is being correctly rendered in my pull request and there is something wrong in the iOS port, maybe it wasn't completely merged or there is something about iOS clipping or rendering that I completely ignore. Please see the attached images below:

[FabricioCabeca]

As a side note, it would be a good idea to implement tiling in the iOS port (it would also fix this issue I believe, android and windows doesn't need any clipping at all). I am not that good with iOS opengl, but I guess it should be piece of cake for you or Steve, I've found a link that can guide us into achieving this: http://iphonedevelopment.blogspot.com.br/2009/05/opengl-es-from-ground-up-part-6_25.html

Also, the clipRect logic in the iOS port is pretty grim, I will try to better understand what is going on there, maybe I can improve it.

[codenameone]

That's a user submitted screenshot so I don't have the code.
Tiling is implemented natively on iOS and has been for ages. Maybe @shannah can help with this.

[shannah]

Why did you take the clipping out of the code that calls the tiling and
move it inside the tileImage() method? Since tileImage() may be overridden
by any platform's port, your addition of the clipping to the tileImage()
method won't be adopted on other platforms (like iOS, Javascript, and
others).

On Sun, Sep 20, 2015 at 8:12 PM, codenameone notifications@github.com
wrote:

That's a user submitted screenshot so I don't have the code.
Tiling is implemented natively on iOS and has been for ages. Maybe
@shannah https://github.com/shannah can help with this.

—
Reply to this email directly or view it on GitHub
#1580 (comment)
.

Steve Hannah
Web Lite Solutions Corp.

[FabricioCabeca]

Hi Steve, that's because the default tileImage needs clipping (otherwise it would paint beyond its specified limits), but platform specific tileImage usually would handle these limits correctly (this is true in the Android submitted tileImage and in my Windows tileImage code), so there would be no need to clip.

Btw, I didn't know there was a native tiling in iOS, but that would explain this edge case, it should be rather easy to fix this by applying the same clipping as I did in the default tileImage.

I believe tileImage should render within its specified limits and the default or overridden tileImage should guarantee that instead of clipping outside, right ?

[FabricioCabeca]

Steve, I am testing a fix for the iOS tileImage clip, I will send you a pull request so you can double check it, ok ?

About other ports, are there any other tileImage implementations ?

[FabricioCabeca]

The fix didn't work, back to the drawing board. Please, let me know if you come up with a solution.

[codenameone]

@shannah can you look at this?

[shannah]

I have some ideas on how to fix this. I'll be able to look into it on Monday

[FabricioCabeca]

Hi, just to let you know I've been just too busy to tackle this one, have you been able to take a look into this ? This patch is a nice improvement even for iOS after tiling is fixed, we're even holding a Pumpop update for now just to have this update, the performance gain in some key screens is quite something ...

[shannah]

Does anyone have a ready to use test case for the iOS problem? I've been able to reproduce some small cases, but nothing as pronounced as the ones posted here.

[FabricioCabeca]

Below is the test case from my screenshot:
https://gist.github.com/FabricioCabeca/2527168d5a2ebfc7c48e

I've used the iOS theme in android to test, you probably only need to use native theme ...

[shannah]

Thanks. Just testing it out now.

On Mon, Sep 28, 2015 at 1:29 PM, FabricioCabeca notifications@github.com
wrote:

Below is the test case from my screenshot:
https://gist.github.com/FabricioCabeca/2527168d5a2ebfc7c48e

—
Reply to this email directly or view it on GitHub
#1580 (comment)
.

Steve Hannah
Web Lite Solutions Corp.

[shannah]

Are the screenshots that you posted earlier using this test case? I'm not seeing the same thing. Running on the iOS simulator in Xcode.

Here is what I see using the revert-1580-master branch:

Can you post the screenshot of what you see?

(Note: I changed isOS7() to return NO so I could get the iOS 6 skin).

[FabricioCabeca]

One thing I have noticed is that there are two implementations when there is extended opengl support or not, in an old iPod Touch it was ok, in an iPhone 5 it was reproducible

[shannah]

OK. I was using the wrong branch. I reproduced the problem and I have submitted a pull request with the changes.
Pmovil#1

[FabricioCabeca]

It seems like it should do the trick, I don't have the vm to generate the ios code here. Were you able to test these changes with the test case ?

[shannah]

Yes. I tested these changes with the test case that you provided and it
seemed to work ok.

On Tue, Sep 29, 2015 at 12:19 PM, FabricioCabeca notifications@github.com
wrote:

It seems like it should do the trick, I don't have the vm to generate the
ios code here. Were you able to test these changes with the test case ?

—
Reply to this email directly or view it on GitHub
#1580 (comment)
.

Steve Hannah
Web Lite Solutions Corp.

[FabricioCabeca]

Should I send another pull request with the merged fix ? Do you want to test more ?

[shannah]

I just decided to do some more testing and ran into an issue on KitchenSink.

I'm going to do some more testing to see if this is iOS specific, or if it is a regression on other platforms too.

[FabricioCabeca]

Ok, I'm setting up the vm here so I can test it.

I didn't understand the DrawImage change, it seems like an old hack to fix the position in the simulator

[shannah]

The DrawImage change was somewhat unrelated but it was necessary to make
things work in the simulator.
I have verified that the problem is iOS specific. I'm revisiting my
strategy. I have a few ideas to do it more efficiently. And hopefully the
result will also fix these glitches.

On Tue, Sep 29, 2015 at 2:13 PM, FabricioCabeca notifications@github.com
wrote:

Ok, I'm setting up the vm here so I can test it.

I didn't understand the DrawImage change, it seems like an old hack to fix
the position in the simulator

—
Reply to this email directly or view it on GitHub
#1580 (comment)
.

Steve Hannah
Web Lite Solutions Corp.

[codenameone]

Which project?

[FabricioCabeca]

The main one - CodenameOne

[codenameone]

Did you look in cn1-binaries?

[FabricioCabeca]

Oops, there it is :-)

[shannah]

Everything looks fine to me now. Fabricio, can you create a new pull request with the latest state of your repo so I can merge it?

[FabricioCabeca]

#1587

Only merged your changes.

[shannah]

OK. Was just about to merge but thought I'd test the master to see if it had the same problem. But it doesn't. Therefore there is still a problem with the tiling on iOS. Here is how it should look with the leather theme.

Here is how it looks with the new tiling

Looking into it more to see if I can identify the cause.

[shannah]

I have prepared a merge locally, but I'm trying to solve this problem first. I believe I have narrowed it down to a problem with EncodedImage on the iOS port (so actually unrelated to this patch), but I would like to fix that issue first.

@codenameone The cause of this problem (or difference) is that the Graphics.tileImage() implementation calls Image.getImage() and passes the result to the implementation's tileImage() method whereas drawImage() simple accesses the Image.image member directly and passes it to the implementation's drawImage() method. Probably the iOS implementation has a problem with EncodedImage getImage() method... perhaps some scaling not being respected... I don't know. Any ideas?

[codenameone]

The iOS just scales by setting numeric flags on NativeImage and that does trigger things...
Accessing the image object directly might be a problem since we do some manipulations in the encoded image class... Its scary to do stuff that's so low level on the metal but I'd like to know if that's a bug...

[FabricioCabeca]

@shannah now that you mention, I do remember once having a problem with images not being scaled as they should only in iOS and only in a specific use case, let me see if I can find it here ...

[FabricioCabeca]

Just remembered it was an issue when applying masks, only in iOS, but it looks like it was fixed, the old code that used to fail is not failing anymore.

[shannah]

OK. I think I'm close. EncodedImage has special handling for drawImage(). It looks like this:

    protected void drawImage(Graphics g, Object nativeGraphics, int x, int y) {
        Display.getInstance().getImplementation().drawingEncodedImage(this);
        Image internal = getInternalImpl();
        if(width > -1 && height > -1 && (internal.getWidth() != width || internal.getHeight() != height)) {
            internal.drawImage(g, nativeGraphics, x, y, width, height);
        } else {
            internal.drawImage(g, nativeGraphics, x, y);
        }
    }

Notice that it handles the case specifically where internal.getWidth() != width and internal.getHeight() != height.

When we call tileImage() it essentially gets "internal" and passes that image directly to be drawn.

@codenameone In what circumstance would internal.getWidth() not equal to width? I think I just need to handle this case in tileImage().

[codenameone]

I think this might relate to the Image.scaled implementation on iOS but I can't recall.

[FabricioCabeca]

EncodedImage can create images passing width and height despite the original size of the binary content, it is just testing this specific use case I guess.

[codenameone]

I think there might be a -1 situation where width/height aren't initialized.

[FabricioCabeca]

Just like there was no clip in place for tileImage, maybe it lacks an affine transformation ? Just a wild guess :-P

[shannah]

Sorry guys... red herring. The problem was in the patch itself. In the implementation of paintBorderBackground(), in the case of TYPE_IMAGE_HORIZONTAL, it was tiling the image over the entire background. However, it should only have tiled a single strip. This didn't show up in other ports because: 1. On Android it used a different title bar so it didn't render this part at all, and 2. In other themes the component (Title area) is actually the same height as the image that is being tiled.

The same bug exists for TYPE_IMAGE_VERTICAL. I'll be fixing these up, and pushing to the master shortly.

[FabricioCabeca]

@shannah also, in the patch you sent me I saw the clipped bool is only set in the width limit check, shouldn't it be set in the height check too ?

https://github.com/Pmovil/CodenameOne/blob/master/Ports/iOSPort/nativeSources/TileImage.m

[shannah]

That was a remnant of a previous commit. I removed it in my local copy.
Now that I have found the problem, I'll be changing the iOS implementation
anyways to use a single call to drawArrays() instead of one for each
tile... this will boost performance more.

On Thu, Oct 1, 2015 at 10:55 AM, FabricioCabeca notifications@github.com
wrote:

@shannah https://github.com/shannah also, in the patch you sent me I
saw the clipped bool is only set in the width limit check, shouldn't it be
set in the height check too ?

https://github.com/Pmovil/CodenameOne/blob/master/Ports/iOSPort/nativeSources/TileImage.m

—
Reply to this email directly or view it on GitHub
#1580 (comment)
.

Steve Hannah
Web Lite Solutions Corp.

[FabricioCabeca]

It should improve a lot to use a single call to drawArrays(), anxious here to test these improvements :-)

[shannah]

I have merged this into the master now.
e955598

[shannah]

Shoot! I was just about to declare victory when I noticed this:

Native dialogs look fine still.

I'm looking through the patch for anything peculiar. I do notice that in a number of places in the original Border implementation, it uses setClipScaled(g, clipX, clipY, clipWidth, clipHeight) to restore the clip after a clipRect() call, but in tileImage() it just uses setClip() to restore the clip.

@codenameone Do you know why it was necessary to use setClipScaled() originally?

[shannah]

Solved it. Posted changes to master. Everything seems to be working correctly now.

[codenameone]

Just FYI we added the setClipScaled because of the "bounce" transition for iOS dialogs where we use scale to make that animation and it broke clipping resulting in bad overlap.

[shannah]

Do you have a test case I can try to make sure that that still works?

On Thu, Oct 1, 2015 at 8:14 PM, codenameone notifications@github.com
wrote:

Just FYI we added the setClipScaled because of the "bounce" transition for
iOS dialogs where we use scale to make that animation and it broke clipping
resulting in bad overlap.

—
Reply to this email directly or view it on GitHub
#1580 (comment)
.

Steve Hannah
Web Lite Solutions Corp.

[codenameone]

Nope.
But if you show the old dialogs on the old iOS 6 theme they should animate "bounce" into place.

[shannah]

Just tried it. Still works :)

On Thu, Oct 1, 2015 at 8:36 PM, codenameone notifications@github.com
wrote:

Nope.
But if you show the old dialogs on the old iOS 6 theme they should animate
"bounce" into place.

—
Reply to this email directly or view it on GitHub
#1580 (comment)
.

Steve Hannah
Web Lite Solutions Corp.

[shannah]

Nevermind. Seems to work OK in simulator too.

On Thu, Oct 1, 2015 at 8:41 PM, Steve Hannah steve@weblite.ca wrote:

On an iOS device, that is. I suspect it won't work in the simulator or on
another device anymore, because the tileImage() implementation just uses
the regular setClip() method to restore the clip.

My iOS implementation doesn't use clipping at all for its tileImage()
implementation... which is why it still works.

On Thu, Oct 1, 2015 at 8:38 PM, Steve Hannah steve@weblite.ca wrote:

Just tried it. Still works :)

On Thu, Oct 1, 2015 at 8:36 PM, codenameone notifications@github.com
wrote:

Nope.
But if you show the old dialogs on the old iOS 6 theme they should
animate "bounce" into place.

—
Reply to this email directly or view it on GitHub
#1580 (comment)
.

Steve Hannah
Web Lite Solutions Corp.

Steve Hannah
Web Lite Solutions Corp.

Steve Hannah
Web Lite Solutions Corp.

[FabricioCabeca]

Thanks for your efforts ! I can see some noticeable performance improvements in our apps comparing to the last ones submitted to stores. Now I will play with the new vm and windows phone, it is getting better and feature complete and this border improvement will make things smoother ;-)