GLES2Dbg: improve protocol and error check after shader upload

Change-Id: I0bd3867e0532e1c64c62db816be31ba0102c7e49
Signed-off-by: David Li <davidxli@google.com>

tools/glesv2debugger/generate_MessageFormatter_java.py

@@ -61,10 +61,10 @@ public class MessageFormatter {
     static String FormatFloats(int count, final ByteBuffer data) {
         if (data.remaining() == 0)
             return "[null]";
+        data.order(SampleView.targetByteOrder);
         String ret = "[";
-        for (int i = 0; i < count; i++)
+        for (int i = 0; i < count; i++) {
-        {
+            ret += Float.intBitsToFloat(data.getInt());
-            ret += Float.intBitsToFloat(Integer.reverseBytes(data.getInt()));
             if (i < count - 1)
                 ret += ", ";
         }
@@ -74,10 +74,10 @@ public class MessageFormatter {
     static String FormatInts(int count, final ByteBuffer data) {
         if (data.remaining() == 0)
             return "[null]";
+        data.order(SampleView.targetByteOrder);
         String ret = "[";
-        for (int i = 0; i < count; i++)
+        for (int i = 0; i < count; i++) {
-        {
+            ret += data.getInt();
-            ret += Integer.reverseBytes(data.getInt());
             if (i < count - 1)
                 ret += ", ";
         }
@@ -87,10 +87,10 @@ public class MessageFormatter {
     static String FormatUints(int count, final ByteBuffer data) {
         if (data.remaining() == 0)
             return "[null]";
+        data.order(SampleView.targetByteOrder);
         String ret = "[";
-        for (int i = 0; i < count; i++)
+        for (int i = 0; i < count; i++) {
-        {
+            long bits = data.getInt() & 0xffffffff;
-            long bits = Integer.reverseBytes(data.getInt()) & 0xffffffff;
             ret += bits;
             if (i < count - 1)
                 ret += ", ";
@@ -101,10 +101,10 @@ public class MessageFormatter {
     static String FormatMatrix(int columns, int count, final ByteBuffer data) {
         if (data.remaining() == 0)
             return "[null]";
+        data.order(SampleView.targetByteOrder);
         String ret = "[";
-        for (int i = 0; i < count; i++)
+        for (int i = 0; i < count; i++) {
-        {
+            ret += Float.intBitsToFloat(data.getInt());
-            ret += Float.intBitsToFloat(Integer.reverseBytes(data.getInt()));
             if (i % columns == columns - 1)
                 ret += "\\n                                             ";
             else if (i < count - 1)

tools/glesv2debugger/src/com/android/glesv2debugger/BreakpointOption.java

@@ -35,6 +35,7 @@ import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Group;
 import org.eclipse.swt.widgets.Shell;
 
+import java.io.IOException;
 import java.util.HashMap;
 
 public class BreakpointOption extends ScrolledComposite implements SelectionListener {
@@ -76,7 +77,7 @@ public class BreakpointOption extends ScrolledComposite implements SelectionList
         Message.Builder builder = Message.newBuilder();
         builder.setContextId(0); // FIXME: proper context id
         builder.setType(Type.Response);
-        builder.setExpectResponse(false);
+        builder.setExpectResponse(true);
         builder.setFunction(Function.SETPROP);
         builder.setProp(Prop.ExpectResponse);
         builder.setArg0(function.getNumber());
@@ -95,7 +96,13 @@ public class BreakpointOption extends ScrolledComposite implements SelectionList
     public void widgetDefaultSelected(SelectionEvent e) {
     }
 
-    public void BreakpointReached(final Message.Builder builder, final Message msg) {
+    private Function lastFunction = Function.NEG;
+
+    public boolean ProcessMessage(final MessageQueue queue, final Message msg) throws IOException {
+        final Message.Builder builder = Message.newBuilder();
+        builder.setContextId(msg.getContextId());
+        builder.setType(Type.Response);
+        builder.setExpectResponse(true);
         final Shell shell = sampleView.getViewSite().getShell();
         shell.getDisplay().syncExec(new Runnable() {
             @Override
@@ -119,6 +126,11 @@ public class BreakpointOption extends ScrolledComposite implements SelectionList
                     call = "continue " + call;
                     builder.setFunction(Function.CONTINUE);
                 }
+                else
+                {
+                    assert msg.getType() == Type.AfterGeneratedCall;
+                    assert msg.getFunction() == lastFunction;
+                }
                 InputDialog inputDialog = new InputDialog(shell,
                             msg.getFunction().toString() + " " + msg.getType().toString(),
                         "(s)kip, (c)continue, (r)emove bp or glFunction(...)",
@@ -137,10 +149,17 @@ public class BreakpointOption extends ScrolledComposite implements SelectionList
                         SetBreakpoint(msg.getFunction(), false);
                     }
                     else
+                    {
                         MessageParserEx.instance.Parse(builder, inputDialog.getValue());
-                    builder.setExpectResponse(true);
+                        lastFunction = builder.getFunction();
                     }
                 }
+                else
+                    assert false; // TODO: cancel behaviour
+                // TODO: add/modify/remove completed messages in queue
+            }
         });
+        queue.SendMessage(builder.build());
+        return true;
     }
 }

tools/glesv2debugger/src/com/android/glesv2debugger/MessageFormatter.java

@@ -25,10 +25,10 @@ public class MessageFormatter {
     static String FormatFloats(int count, final ByteBuffer data) {
         if (data.remaining() == 0)
             return "[null]";
+        data.order(SampleView.targetByteOrder);
         String ret = "[";
-        for (int i = 0; i < count; i++)
+        for (int i = 0; i < count; i++) {
-        {
+            ret += Float.intBitsToFloat(data.getInt());
-            ret += Float.intBitsToFloat(Integer.reverseBytes(data.getInt()));
             if (i < count - 1)
                 ret += ", ";
         }
@@ -38,10 +38,10 @@ public class MessageFormatter {
     static String FormatInts(int count, final ByteBuffer data) {
         if (data.remaining() == 0)
             return "[null]";
+        data.order(SampleView.targetByteOrder);
         String ret = "[";
-        for (int i = 0; i < count; i++)
+        for (int i = 0; i < count; i++) {
-        {
+            ret += data.getInt();
-            ret += Integer.reverseBytes(data.getInt());
             if (i < count - 1)
                 ret += ", ";
         }
@@ -51,10 +51,10 @@ public class MessageFormatter {
     static String FormatUints(int count, final ByteBuffer data) {
         if (data.remaining() == 0)
             return "[null]";
+        data.order(SampleView.targetByteOrder);
         String ret = "[";
-        for (int i = 0; i < count; i++)
+        for (int i = 0; i < count; i++) {
-        {
+            long bits = data.getInt() & 0xffffffff;
-            long bits = Integer.reverseBytes(data.getInt()) & 0xffffffff;
             ret += bits;
             if (i < count - 1)
                 ret += ", ";
@@ -65,10 +65,10 @@ public class MessageFormatter {
     static String FormatMatrix(int columns, int count, final ByteBuffer data) {
         if (data.remaining() == 0)
             return "[null]";
+        data.order(SampleView.targetByteOrder);
         String ret = "[";
-        for (int i = 0; i < count; i++)
+        for (int i = 0; i < count; i++) {
-        {
+            ret += Float.intBitsToFloat(data.getInt());
-            ret += Float.intBitsToFloat(Integer.reverseBytes(data.getInt()));
             if (i % columns == columns - 1)
                 ret += "\n                                             ";
             else if (i < count - 1)

tools/glesv2debugger/src/com/android/glesv2debugger/MessageQueue.java

@@ -32,8 +32,8 @@ public class MessageQueue implements Runnable {
 
     boolean running = false;
     Thread thread = null;
-    ArrayList<Message> complete = new ArrayList<Message>();
+    ArrayList<Message> complete = new ArrayList<Message>(); // need synchronized
-    ArrayList<Message> commands = new ArrayList<Message>();
+    ArrayList<Message> commands = new ArrayList<Message>(); // need synchronized
     SampleView sampleView;
 
     public MessageQueue(SampleView sampleView) {
@@ -58,21 +58,16 @@ public class MessageQueue implements Runnable {
         return running;
     }
 
-    boolean SendCommands(final DataOutputStream dos, final int contextId) throws IOException {
+    void SendCommands(final int contextId) throws IOException {
-        boolean sent = false;
         synchronized (commands) {
             for (int i = 0; i < commands.size(); i++) {
                 Message command = commands.get(i);
-                // FIXME: proper context id
+                if (command.getContextId() == contextId || command.getContextId() == 0) {
-                if (command.getContextId() == contextId || contextId == 0) {
+                    SendMessage(commands.remove(i));
-                    SendMessage(dos, command);
-                    commands.remove(i);
                     i--;
-                    sent = true;
                 }
             }
         }
-        return sent;
     }
 
     public void AddCommand(Message command) {
@@ -81,18 +76,15 @@ public class MessageQueue implements Runnable {
         }
     }
 
-    public void AddCommands(ArrayList<Message> cmds) {
+    // these should only be accessed from the network thread;
-        synchronized (commands) {
+    // access call chain starts with run()
-            commands.addAll(cmds);
+    private DataInputStream dis = null;
-        }
+    private DataOutputStream dos = null;
-    }
+    private HashMap<Integer, ArrayList<Message>> incoming = new HashMap<Integer, ArrayList<Message>>();
 
     @Override
     public void run() {
         Socket socket = new Socket();
-        DataInputStream dis = null;
-        DataOutputStream dos = null;
-        HashMap<Integer, ArrayList<Message>> incoming = new HashMap<Integer, ArrayList<Message>>();
         try {
             socket.connect(new java.net.InetSocketAddress("127.0.0.1", Integer
                     .parseInt(sampleView.actionPort.getText())));
@@ -106,18 +98,16 @@ public class MessageQueue implements Runnable {
         while (running) {
             Message msg = null;
             if (incoming.size() > 0) { // find queued incoming
-                for (ArrayList<Message> messages : incoming
+                for (ArrayList<Message> messages : incoming.values())
-                            .values())
                     if (messages.size() > 0) {
-                        msg = messages.get(0);
+                        msg = messages.remove(0);
-                        messages.remove(0);
                         break;
                     }
             }
-            if (null == msg) { // get incoming from network
             try {
-                    msg = ReadMessage(dis);
+                if (null == msg) // get incoming from network
-                    SendResponse(dos, msg);
+                    msg = ReceiveMessage(dis);
+                ProcessMessage(dos, msg);
             } catch (IOException e) {
                 Error(e);
                 running = false;
@@ -125,57 +115,6 @@ public class MessageQueue implements Runnable {
             }
         }
 
-            int contextId = msg.getContextId();
-            if (!incoming.containsKey(contextId))
-                incoming.put(contextId, new ArrayList<Message>());
-
-            if (msg.getType() == Type.AfterGeneratedCall)
-                continue; // TODO: for now, don't care
-
-            // FIXME: the expected sequence will change for interactive mode
-            while (msg.getType() == Type.BeforeCall) {
-                Message next = null;
-                // get existing message part for this context
-                ArrayList<Message> messages = incoming
-                            .get(contextId);
-                if (messages.size() > 0) {
-                    next = messages.get(0);
-                    messages.remove(0);
-                }
-                if (null == next) { // read new part for message
-                    try {
-                        next = ReadMessage(dis);
-                        SendResponse(dos, next);
-                    } catch (IOException e) {
-                        Error(e);
-                        running = false;
-                        break;
-                    }
-
-                    if (next.getType() == Type.AfterGeneratedCall)
-                        continue; // TODO: for now, don't care
-
-                    if (next.getContextId() != contextId) {
-                        // message part not for this context
-                        if (!incoming.containsKey(next.getContextId()))
-                            incoming.put(
-                                        next.getContextId(),
-                                        new ArrayList<Message>());
-                        incoming.get(next.getContextId()).add(next);
-                        continue;
-                    }
-                }
-
-                Message.Builder builder = msg.toBuilder();
-                builder.mergeFrom(next);
-                msg = builder.build();
-            }
-
-            synchronized (complete) {
-                complete.add(msg);
-            }
-        }
-
         try {
             socket.close();
         } catch (IOException e) {
@@ -184,9 +123,64 @@ public class MessageQueue implements Runnable {
         }
     }
 
-    Message GetMessage(int contextId) {
+    private void PutMessage(final Message msg) {
-        // ReadMessage and filter by contextId
+        ArrayList<Message> existing = incoming.get(msg.getContextId());
-        return null;
+        if (existing == null)
+            incoming.put(msg.getContextId(), existing = new ArrayList<Message>());
+        existing.add(msg);
+    }
+
+    Message ReceiveMessage(final int contextId) throws IOException {
+        Message msg = ReceiveMessage(dis);
+        while (msg.getContextId() != contextId) {
+            PutMessage(msg);
+            msg = ReceiveMessage(dis);
+        }
+        return msg;
+    }
+
+    void SendMessage(final Message msg) throws IOException {
+        SendMessage(dos, msg);
+    }
+
+    // should only used by DefaultProcessMessage
+    private HashMap<Integer, Message> partials = new HashMap<Integer, Message>();
+
+    Message GetPartialMessage(final int contextId) {
+        return partials.get(contextId);
+    }
+
+    // can be used by other message processor as default processor
+    void DefaultProcessMessage(final Message msg, boolean expectResponse,
+            boolean sendResponse)
+            throws IOException {
+        assert !msg.getExpectResponse();
+        final int contextId = msg.getContextId();
+        final Message.Builder builder = Message.newBuilder();
+        builder.setContextId(contextId);
+        builder.setType(Type.Response);
+        builder.setExpectResponse(expectResponse);
+        if (msg.getType() == Type.BeforeCall) {
+            if (sendResponse) {
+                builder.setFunction(Function.CONTINUE);
+                SendMessage(dos, builder.build());
+            }
+            assert !partials.containsKey(contextId);
+            partials.put(contextId, msg);
+        } else if (msg.getType() == Type.AfterCall) {
+            if (sendResponse) {
+                builder.setFunction(Function.CONTINUE);
+                SendMessage(dos, builder.build());
+            }
+            assert partials.containsKey(contextId);
+            final Message before = partials.remove(contextId);
+            assert before.getFunction() == msg.getFunction();
+            final Message completed = before.toBuilder().mergeFrom(msg).build();
+            synchronized (complete) {
+                complete.add(completed);
+            }
+        } else
+            assert false;
     }
 
     public Message RemoveCompleteMessage(int contextId) {
@@ -194,11 +188,7 @@ public class MessageQueue implements Runnable {
             if (complete.size() == 0)
                 return null;
             if (0 == contextId) // get a message for any context
-            {
+                return complete.remove(0);
-                Message msg = complete.get(0);
-                complete.remove(0);
-                return msg;
-            }
             for (int i = 0; i < complete.size(); i++) {
                 Message msg = complete.get(i);
                 if (msg.getContextId() == contextId) {
@@ -210,7 +200,7 @@ public class MessageQueue implements Runnable {
         return null;
     }
 
-    Message ReadMessage(final DataInputStream dis)
+    private Message ReceiveMessage(final DataInputStream dis)
             throws IOException {
         int len = 0;
         try {
@@ -234,33 +224,27 @@ public class MessageQueue implements Runnable {
                 readLen += read;
         }
         Message msg = Message.parseFrom(buffer);
+        SendCommands(msg.getContextId());
         return msg;
     }
 
-    void SendMessage(final DataOutputStream dos, final Message message)
+    private void SendMessage(final DataOutputStream dos, final Message message)
             throws IOException {
         final byte[] data = message.toByteArray();
         dos.writeInt(data.length);
         dos.write(data);
     }
 
-    void SendResponse(final DataOutputStream dos, final Message msg) throws IOException {
+    private void ProcessMessage(final DataOutputStream dos, final Message msg) throws IOException {
-        final Message.Builder builder = Message.newBuilder();
+        if (msg.getExpectResponse()) {
-        builder.setContextId(msg.getContextId());
+            if (sampleView.shaderEditor.ProcessMessage(this, msg))
-        if (msg.getType() == Type.BeforeCall)
+                return;
-            builder.setFunction(Function.CONTINUE);
+            else if (sampleView.breakpointOption.ProcessMessage(this, msg))
-        else if (msg.getType() == Type.AfterCall)
+                return;
-            builder.setFunction(Function.SKIP);
-        else if (msg.getType() == Type.AfterGeneratedCall)
-            builder.setFunction(Function.SKIP);
             else
-            assert false;
+                DefaultProcessMessage(msg, msg.getExpectResponse(), msg.getExpectResponse());
-        builder.setType(Type.Response);
+        } else
-        builder.setExpectResponse(msg.getExpectResponse());
+            DefaultProcessMessage(msg, msg.getExpectResponse(), msg.getExpectResponse());
-        if (msg.getExpectResponse())
-            sampleView.breakpointOption.BreakpointReached(builder, msg);
-        if (SendCommands(dos, 0) || msg.getExpectResponse())
-            SendMessage(dos, builder.build());
     }
 
     void Error(Exception e) {

tools/glesv2debugger/src/com/android/glesv2debugger/ShaderEditor.java

@@ -17,6 +17,7 @@
 package com.android.glesv2debugger;
 
 import com.android.glesv2debugger.DebuggerMessage.Message;
+import com.android.glesv2debugger.DebuggerMessage.Message.Function;
 import com.android.glesv2debugger.DebuggerMessage.Message.Type;
 
 import org.eclipse.jface.dialogs.MessageDialog;
@@ -35,6 +36,7 @@ import org.eclipse.swt.widgets.List;
 import org.eclipse.swt.widgets.ToolBar;
 import org.eclipse.swt.widgets.ToolItem;
 
+import java.io.IOException;
 import java.util.ArrayList;
 
 public class ShaderEditor extends Composite implements SelectionListener, ExtendedModifyListener {
@@ -47,6 +49,8 @@ public class ShaderEditor extends Composite implements SelectionListener, Extend
 
     GLShader current;
 
+    ArrayList<GLShader> shadersToUpload = new ArrayList<GLShader>();
+
     ArrayList<Message> cmds = new ArrayList<Message>();
 
     ShaderEditor(SampleView sampleView, Composite parent) {
@@ -100,12 +104,12 @@ public class ShaderEditor extends Composite implements SelectionListener, Extend
                 builder.append(shader.name);
                 while (builder.length() < 40)
                     builder.append(" ");
-                builder.append(':');
+                builder.append(" : ");
                 for (Context ctx : context.shares) {
                     builder.append(String.format("%08X", ctx.contextId));
                     builder.append(' ');
                 }
-                builder.append(':');
+                builder.append(": ");
                 for (GLProgram program : shader.programs) {
                     builder.append(program.name);
                     builder.append(" ");
@@ -118,37 +122,126 @@ public class ShaderEditor extends Composite implements SelectionListener, Extend
     void UploadShader() {
         current.source = styledText.getText();
 
-        ArrayList<Message> cmds = new ArrayList<Message>();
+        // add the initial command, which when read by server will set
+        // expectResponse for the message loop and go into message exchange
+        synchronized (shadersToUpload) {
+            if (shadersToUpload.size() > 0) {
+                MessageDialog.openWarning(this.getShell(), "",
+                        "Previous shader upload not complete, try again");
+                return;
+            }
+            shadersToUpload.add(current);
             final int contextId = current.context.context.contextId;
-
             Message.Builder builder = GetBuilder(contextId);
             MessageParserEx.instance.Parse(builder,
                     String.format("glShaderSource(%d,1,\"%s\",0)", current.name, current.source));
-        cmds.add(builder.build());
+            sampleView.messageQueue.AddCommand(builder.build());
-
-        builder = GetBuilder(contextId);
-        MessageParserEx.instance.Parse(builder,
-                String.format("glCompileShader(%d)", current.name));
-        cmds.add(builder.build());
-
-        for (GLProgram program : current.programs) {
-            builder = GetBuilder(contextId);
-            MessageParserEx.instance.Parse(builder,
-                    String.format("glLinkProgram(%d)", program.name));
-            cmds.add(builder.build());
         }
-
-        sampleView.messageQueue.AddCommands(cmds);
     }
 
     Message.Builder GetBuilder(int contextId) {
         Message.Builder builder = Message.newBuilder();
         builder.setContextId(contextId);
         builder.setType(Type.Response);
-        builder.setExpectResponse(false);
+        builder.setExpectResponse(true);
         return builder;
     }
 
+    Message ExchangeMessage(final int contextId, final MessageQueue queue,
+            String format, Object... args) throws IOException {
+        Message.Builder builder = GetBuilder(contextId);
+        MessageParserEx.instance.Parse(builder, String.format(format, args));
+        final Function function = builder.getFunction();
+        queue.SendMessage(builder.build());
+        final Message msg = queue.ReceiveMessage(contextId);
+        assert msg.getContextId() == contextId;
+        assert msg.getType() == Type.AfterGeneratedCall;
+        assert msg.getFunction() == function;
+        return msg;
+    }
+
+    // this is called from network thread
+    public boolean ProcessMessage(final MessageQueue queue, final Message msg)
+            throws IOException {
+        GLShader shader = null;
+        final int contextId = msg.getContextId();
+        synchronized (shadersToUpload) {
+            if (shadersToUpload.size() == 0)
+                return false;
+            shader = shadersToUpload.get(0);
+            boolean matchingContext = false;
+            for (Context ctx : shader.context.context.shares)
+                if (ctx.contextId == msg.getContextId()) {
+                    matchingContext = true;
+                    break;
+                }
+            if (!matchingContext)
+                return false;
+            shadersToUpload.remove(0);
+        }
+
+        // glShaderSource was already sent to trigger set expectResponse
+        assert msg.getType() == Type.AfterGeneratedCall;
+        assert msg.getFunction() == Function.glShaderSource;
+
+        ExchangeMessage(contextId, queue, "glCompileShader(%d)", shader.name);
+
+        // the 0, "" and [0] are dummies for the parser
+        Message rcv = ExchangeMessage(contextId, queue,
+                "glGetShaderiv(%d, GL_COMPILE_STATUS, [0])", shader.name);
+        assert rcv.hasData();
+        if (rcv.getData().asReadOnlyByteBuffer().getInt() == 0) {
+            // compile failed
+            rcv = ExchangeMessage(contextId, queue,
+                    "glGetShaderInfoLog(%d, 0, 0, \"\")", shader.name);
+            final String title = String.format("Shader %d in 0x%s failed to compile",
+                    shader.name, Integer.toHexString(shader.context.context.contextId));
+            final String message = rcv.getData().toStringUtf8();
+            sampleView.getSite().getShell().getDisplay().syncExec(new Runnable() {
+                @Override
+                public void run()
+                {
+                    MessageDialog.openWarning(getShell(), title, message);
+                }
+            });
+        } else
+            for (GLProgram program : shader.programs) {
+                ExchangeMessage(contextId, queue, "glLinkProgram(%d)", program.name);
+                rcv = ExchangeMessage(contextId, queue,
+                        "glGetProgramiv(%d, GL_LINK_STATUS, [0])", program.name);
+                assert rcv.hasData();
+                if (rcv.getData().asReadOnlyByteBuffer().getInt() != 0)
+                    continue;
+                // link failed
+                rcv = ExchangeMessage(contextId, queue,
+                            "glGetProgramInfoLog(%d, 0, 0, \"\")", program.name);
+                final String title = String.format("Program %d in 0x%s failed to link",
+                        program.name, Integer.toHexString(program.context.context.contextId));
+                final String message = rcv.getData().toStringUtf8();
+                sampleView.getSite().getShell().getDisplay().syncExec(new Runnable() {
+                    @Override
+                    public void run()
+                    {
+                        MessageDialog.openWarning(getShell(), title, message);
+                    }
+                });
+                // break;
+            }
+
+        // TODO: add to upload results if failed
+
+        Message.Builder builder = GetBuilder(contextId);
+        builder.setExpectResponse(false);
+        if (queue.GetPartialMessage(contextId) != null)
+            // the glShaderSource interrupted a BeforeCall, so continue
+            builder.setFunction(Function.CONTINUE);
+        else
+            builder.setFunction(Function.SKIP);
+        queue.SendMessage(builder.build());
+
+        return true;
+    }
+
     @Override
     public void widgetSelected(SelectionEvent e) {
         if (e.getSource() == uploadShader && null != current) {