JDK-8117362 : Viewport is not handled correctly on devices that lack non-power-of-2 texture support
  • Type: Bug
  • Component: javafx
  • Sub-Component: graphics
  • Affected Version: 8
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2013-05-04
  • Updated: 2015-06-17
  • Resolved: 2013-08-08
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8
8Fixed
Related Reports
Blocks :  
JDK-8097319 - Quantum Painters need to use readPixels correctly.
Blocks :  
JDK-8095862 - Webkit RTImage needs to use width/height correctly for readPixels.
Relates :  
JDK-8093993 - Camera regression on devices that lack non-power-of-2 texture support
Relates :  
JDK-8124960 - Oversized and incomplete components shown on osx 10.8.3 using jdk 1.8.0
Relates :  
JDK-8102550 - Need to Evaluate ES2 implementation of contentWidth/Height and RTT Interface.
Relates :  
JDK-8123020 - Resizing a Window causes the size of objects to jitter
Description
While fixing RT-29954, we again ran into a problem rendering into an RTT whose content size is bigger than the viewport into which we want to render. A partial fix (more of a workaround, really) was implemented in the ES2 pipeline to allow ParallelCamera to work for for systems without NPOT support -- specifically BeagleBoard.

This solution is incomplete in that it doesn't work correctly for PerspectiveCamera. Also, by setting the forcepowerof2 flag, I verified that if we ever find a D3D system without NPOT suport, it would break as well.

A longer term solution should involve having the RTT itself be aware of the viewport into which it is rendered. Note that this is an issue that affects more than just the camera transforms. See RT-26216, RT-26217, RT-26218 for other examples of why not having that information is a problem. Note that there could be other cases besides lack of NPOT support where an RTT could be bigger than the viewport into which you are rendering (e.g., reusing an RTT that is bigger than you need to avoid recreating it). It is possible we just haven't hit those cases yet in testing on BeagleBoard.
Comments
Changeset: 1222358a4d4d Author: "Joseph Andresen<joseph.andresen@oracle.com>" Date: 2013-08-08 11:31 -0700 URL: http://hg.openjdk.java.net/openjfx/8/graphics/rt/rev/1222358a4d4d Tested with ColorCube project
08-08-2013
Another Area in NGRegion which is likely broken now. if (cached != null) { final double dstWidth = width + roundUp(outsets.getLeft()) + roundUp(outsets.getRight()); if (cached.getContentWidth() == dstWidth) {
25-06-2013
ImagePool Code: // first look for an already cached image of sufficient size, // choosing the one that is closest in size to the requested dimensions SoftReference<Filterable> chosenEntry = null; Filterable chosenImage = null; int mindiff = Integer.MAX_VALUE; Iterator<SoftReference<Filterable>> entries = unlocked.iterator(); while (entries.hasNext()) { SoftReference<Filterable> entry = entries.next(); Filterable eimg = entry.get(); if (eimg == null) { entries.remove(); continue; } int ew = eimg.getContentWidth(); int eh = eimg.getContentHeight(); There is one use... ill check the other spots
25-06-2013
I know that ImagePool does. Areas that someone should check to see if they recycle would be Node caching, CachingShapeRep, Region caching. There may be others, I'm not sure I would know of the exhaustive list of "where the code tries to reuse RTTs"...
24-06-2013
So i need to test the semantics of these variables then. Is there a good test that recycles textures?
24-06-2013
The problem is that it is hard to "know" something like this. It's not very clear that ImagePool was depending on that relationship - thus the comment in ES2RTT... ...jim On 5/8/13 4:50 PM, Kevin Rushforth wrote: > Which, if I had read Jim's follow-on e-mail before sending this, is I > think what he was recommending. > > -- Kevin > > > Kevin Rushforth wrote: >> Maybe a cleaner fix then would be to make contentWidth and >> contentHeight mean the size we requested and want to render into. We >> wouldn't need to introduce viewportWidth / viewportHeight. We would >> need to validate that no other place cares that contentSize == >> physicalSize-padding, but it may end up being a less-intrusive and >> more intuitive change. We could introduce a maxContentSize if needed. >> >> -- Kevin
24-06-2013
Here is another way to look at it. We have the actual number of pixels allocated to a texture - that is physWH. We have the number of pixels someone was playing with (or intending to play with) - that should be contentWH, but we have exceptions. The platform may require us to do something special with some of the pixels in the physical texture - for example, leave a border of transparent pixels for CLAMP_TO_ZERO simulation for RTTs. Simulating CLAMP_TO_EDGE and REPEAT also happens for regular (non-RTT) textures. In either case we have no way to query that. We have the maximum size that the user is allowed to "play with" which is physWH minus the pixels that need to be used for simulation purposes. There is no definitive way to query that, but ES2RTT happens to set contentWH on RTTs to be that number. We could add getMaximumContentWH() so that someone could ask "if I wanted to, how many pixels could I muck with", but as I pointed out - with the fix for 30106 I don't think anyone needs to ask that any more. Still, it might be nice to have something like that and teach other parts of the system to check if their scratch texture can be reused (such as Node.cache - if the node grows slightly it could check if the new size is still within the max content dims of the existing texture and change its contentWH to match rather than reallocate). At some point I think we are going to get rid of ImagePool and all of the random uses of Effect.getCompatImage() and combine them all into a Prism "cached scratch texture" mechanism - probably when I get around to modifying Decora to just use Prism interfaces directly... ...jim
24-06-2013
Ignore that first sentence fragment, I switched to inline replies and forgot to remove it. Note that with the fix for 30106 the ImagePool now requests the "size it was going to get anyway" and so for its textures we do end up with "content dims == size that ImagePool requested", but it may be more than "size that getCompatibleImage caller requested" (and that will always be the case since getCI() promises to return any texture that is at least that size from the pool). And, as it turns out, I think it may have been the only piece of code in the system that cared about that distinction so the "content width == physical width less simulation padding" may not be important to anyone any more...
24-06-2013
Jim said: I think it would work better if we could On 5/8/13 3:07 PM, Joseph Andresen wrote: > in other/simpler terms "Content size is the size which you requested" That's a simple view that it would be nice if we could support. > In ES2RTT it is defined as > "Since > // RTTextures are frequently recycled (i.e., reused by Decora) it > // is imperative that we initialize the content region of the > // RTTexture to be *the physical size of the FBO modulo the > padded** > ** // region.* (Suppose the caller asks for a 110x220 RTTexture, > but (*what does that mean*) > // nonpow2 textures aren't supported; in this case, we will > actually > // create a 128x256 FBO. If later that RTTexture gets reused for > // a caller expecting to use 126x240 pixels, the viewport will be > // setup correctly because it will set to the content region, or > // 128x256 in this case, assuming no padding.)" > > Im confused what this means in ES2RTT... since in the first use content > width is the same as requested, and in the next use it is not equal. if > you are truly needing to know if you can reuse a texture couldnt you > just check This is exactly the cause of the recent RT-30106 fire drill. I think it is misworded. If you request a 110x220 RTT and receive a 128x256 RTT instead and you put it in the cache as a 110x220 then if you later need a 111x220 texture it will assume that it needs to make a new one. But, it should be able to reuse the one it got earlier for 110x220. > physicalSize - padding > sizeRequested? That depends on what you call padding. pow2 can cause us to pad out to a power of 2 with pixels that could be used, but on embedded we have to simulate CLAMP_TO_ZERO with an intentional ring of transparent pixels. > And doesn't that free up content size to always mean when Texture > defines it as? > Regardless of this answer... Can we agree that Texture and ES2RTT define > ContentSize differently? The problem is that we can't redefine the content size of an RTT. I'm not sure what the ramifications might be. It may be simply that an old Graphics would have the wrong clip size, but as long as we identify that you must create a new Graphics if you change the content size then we could add settable content sizes for use in specific situations. So, if ES2RTT did not upgrade content size to physical size (less intentional padding) then we wouldn't have a way for ImagePool to reuse its excess free pixels. > More importantly, if content size was left alone and viewport size > added, viewport size would always be <= to content size AND also equal > to the requested size? I'm not sure where viewport size is coming from so I can't answer that question... ...jim
24-06-2013
Here is a conversation from email that is important for this bug. I'm trying to establish rules before implementing these new terms and I'm not convinced the current names are valid. First off, we can all agree that the total width and height of a texture is its "Physical Size". We call it Physical because the low level texture physically takes up a total amount of texture memory. Content Size is defined in Texture.java as "Note that the * content size of a texture may be smaller than the requested size due * to hardware restrictions (e.g. lack of non-power-of-two texture support). * The content height will be less than or equal to the content (assuming we meant physical) height. * <p> * For example, if the hardware does not support non-power-of-two textures, * and you call ResourceFactory.createTexture(400, 200), the returned * Texture will have physical dimensions of 512x256, but the content * dimensions will be 400x200." in other/simpler terms "Content size is the size which you requested" In ES2RTT it is defined as "Since // RTTextures are frequently recycled (i.e., reused by Decora) it // is imperative that we initialize the content region of the // RTTexture to be the physical size of the FBO modulo the padded // region. (Suppose the caller asks for a 110x220 RTTexture, but (what does that mean) // nonpow2 textures aren't supported; in this case, we will actually // create a 128x256 FBO. If later that RTTexture gets reused for // a caller expecting to use 126x240 pixels, the viewport will be // setup correctly because it will set to the content region, or // 128x256 in this case, assuming no padding.)" Im confused what this means in ES2RTT... since in the first use content width is the same as requested, and in the next use it is not equal. if you are truly needing to know if you can reuse a texture couldnt you just check physicalSize - padding > sizeRequested? And doesn't that free up content size to always mean when Texture defines it as? Regardless of this answer... Can we agree that Texture and ES2RTT define ContentSize differently? More importantly, if content size was left alone and viewport size added, viewport size would always be <= to content size AND also equal to the requested size? -J
24-06-2013
In order to fix retina display (RT-30272), the code quoted above was changed to: // TODO: verify that this is the right solution. There may be // other use-cases where rendering needs different viewport size. double vw = camera.getViewWidth(); double vh = camera.getViewHeight(); if (w != vw || h != vh) { scratchTx.scale(vw / w, vh / h, 1.0); } This works for both retina 2xscaled displays and power-of-2 targets, but it still does not handle perspective transforms correctly.
15-05-2013
The following code was added as a partial fix for RT-29954: // TODO: verify that this is the right solution. There may be // other use-cases where rendering needs different viewport size. if (!glContext.canCreateNonPowTwoTextures()) { powerOfTwoCompensation.setToScale( camera.getViewWidth() / w, camera.getViewHeight() / h, 1.0); scratchTx.mul(powerOfTwoCompensation); } It doesn't work for perspective (I had thought of switching the order of the multiplication as is done for the flip, but Pavel tried it and it didn't seem to work; maybe it needs a pivot point?). Note: assuming that the above code remains after the fix for this issue (which may very well not be the case), then the "!glContext.canCreateNonPowTwoTextures()" test should be replaced with a check for w,h != contentW,H both to avoid the unnecessary transform and also to handle any other cases where the scale should be other than 1. As noted above, this issue could affect the D3D pipeline, too, although there may no longer be any supported platforms that fail to provide NPOT support (note that the comments in RT-26216 imply that D3D doesn't have quite the same treatment of content Width / Height as OpenGL). It seems to me that it will be best to deal with the underlying problem of the viewport being different from what is expected. To this end, I recommend that we add a viewportWidth and viewportHeight to RTT that will define the viewport for rendering, and also will define the effective size of the RTT (since content width/height cannot be used for that purpose).
05-05-2013