Bugs in OpenGLGraphicsContextCustomShader

I'm loving the friendliness of the OpenGLGraphicsContextCustomShader class, but I ran into two issues, both related to the pixelPos shader variable. (This is on Mac, not sure about Windows.)

1. This class automatically creates a varying vec2 called "pixelPos" that is available to your shader. But the x and y values of pixelPos seem to be relative to the window, not the component being drawn in. Is this expected, or a bug? When you call OpenGLGraphicsContextCustomShader::fillRect, it takes a rect in the component's local coordinates, but the shader's pixelPos is in window coordinates.

You can see this in the JUCE demo app. Go to the OpenGL 2D demo, select the Circle shader. If you edit the shader and try to set the center at 0, 0, you'll see the circle goes behind the demo list on the left side of the window.

2. Also related to the pixelPos variable. I don't think this variable is handling Retina displays correctly. JUCE does the right thing by abstracting away Retina pixel coordinates. But if you have a Retina Mac, again go to the OpenGL 2D demo, and you'll see that the circle is positioned differently, and with a different radius than than if you run it in non-Retina mode.

OpenGLGraphicsContextCustomShader abstracts away the Retina situation. You call its fillRect member function the same way whether you're on a Retina screen or not, but the shader gets pixel coordinates that don't match up with the rectangle passed to fillRect. I know OpenGL is not naturally Retina-savvy, but maybe the right thing to do is for JUCE to compensate for Retina when setting up the pixelPos variable?

I concur with the above statements. I've just discovered this for myself, and was looking for a solution and saw this. 

Thanks for the heads-up, guys, I'll look into that..

While you're on the case, I'm seeing this, if I'm using OpenGLGraphicsContextCustomShader and the UI is closed then re-opened quickly. This is common to all hosts and formats.

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib            0x00007fff83155282 __pthread_kill + 10
1   libsystem_c.dylib                 0x00007fff8f110c13 __abort + 145
2   libsystem_c.dylib                 0x00007fff8f110b82 abort + 144
3   com.apple.logic.pro               0x00000001003b3226 std::_Rb_tree<short, std::pair<short const, CGRect>, std::_Select1st<std::pair<short const, CGRect> >, std::less<short>, std::allocator<std::pair<short const, CGRect> > >::_M_erase(std::_Rb_tree_node<std::pair<short const, CGRect> >*) + 98486
4   libsystem_platform.dylib          0x00007fff8c050f1a _sigtramp + 26
5   com.audiodamage.Ricochet2         0x0000000121ab8cc3 juce::OpenGLContext::getCachedImage() const + 15
6   com.audiodamage.Ricochet2         0x0000000121ab8120 juce::OpenGLContext::setAssociatedObject(char const*, juce::ReferenceCountedObject*) + 28
7   com.audiodamage.Ricochet2         0x0000000121ab80ce juce::OpenGLGraphicsContextCustomShader::~OpenGLGraphicsContextCustomShader() + 58

My use of OpenGLGraphicsContextCustomShader is copied directly from the JUCE Demo. 

Here's a slightly different permutation of the same crash. (This is from a 32-bit VST host, as opposed to the one above, which is from 64-bit Logic X.) 

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.audiodamage.Ricochet2         0x0e446372 juce::OpenGLContext::getCachedImage() const + 24
1   com.audiodamage.Ricochet2         0x0e445642 juce::OpenGLContext::setAssociatedObject(char const*, juce::ReferenceCountedObject*) + 20
2   com.audiodamage.Ricochet2         0x0e4455ff juce::OpenGLGraphicsContextCustomShader::~OpenGLGraphicsContextCustomShader() + 69
3   ???                               0x023052c4 0 + 36721348
4   com.audiodamage.Ricochet2         0x0e38382b juce::ContainerDeletePolicy<juce::OpenGLGraphicsContextCustomShader>::destroy(juce::OpenGLGraphicsContextCustomShader*) + 21
5   com.audiodamage.Ricochet2         0x0e382d9b Ricochet2AudioProcessorEditor::~Ricochet2AudioProcessorEditor() + 87
6   com.audiodamage.Ricochet2         0x0e382d0f Ricochet2AudioProcessorEditor::~Ricochet2AudioProcessorEditor() + 17
7   com.audiodamage.Ricochet2         0x0e3eca6c juce::Component::deleteAllChildren() + 20
8   com.audiodamage.Ricochet2         0x0e46609e JuceVSTWrapper::EditorCompWrapper::~EditorCompWrapper() + 42
9   ???                               0x00b49a20 0 + 11835936

This is right, and many things are not thought correctly either.

You don't need pixelPos at all, since you have gl_FragCoord which contains the same information (but right on Retina display) (upside down: 0.5x0.5 is bottom left, (width-.5)x(height-.5) is top right).

A fragmentSize containing the fragment's size would be useful to map a scale in [0 1] range (instead of speaking in pixels in a fragment). You'll need to map to the size of the rectangle given in fillRect method.

However, if you want to bind a texture, the current code asserts everywhere.

I don't want to hijack this thread, so I've posted a new topic about the other bugs I've found in the class usage.