8024395: Improve fix for line break calculations
authordmarkov
Thu, 12 Sep 2013 18:44:14 +0400
changeset 20126 e09648d4170c
parent 20125 65a1e01378a2
child 20127 a3a34675d847
8024395: Improve fix for line break calculations Reviewed-by: alexp, alexsch
jdk/src/share/classes/javax/swing/text/FlowView.java
jdk/src/share/classes/javax/swing/text/View.java
jdk/test/javax/swing/text/View/8014863/bug8014863.java
--- a/jdk/src/share/classes/javax/swing/text/FlowView.java	Thu Sep 12 18:21:06 2013 +0400
+++ b/jdk/src/share/classes/javax/swing/text/FlowView.java	Thu Sep 12 18:44:14 2013 +0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2008, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -796,6 +796,22 @@
             v.setParent(parent);
         }
 
+        /** {@inheritDoc} */
+        @Override
+        protected void forwardUpdate(DocumentEvent.ElementChange ec,
+                                          DocumentEvent e, Shape a, ViewFactory f) {
+            calculateUpdateIndexes(e);
+            // Send update event to all views followed by the changed place.
+            lastUpdateIndex = Math.max((getViewCount() - 1), 0);
+            for (int i = firstUpdateIndex; i <= lastUpdateIndex; i++) {
+                View v = getView(i);
+                if (v != null) {
+                    Shape childAlloc = getChildAllocation(i, a);
+                    forwardUpdateToView(v, e, childAlloc, f);
+                }
+            }
+        }
+
         // The following methods don't do anything useful, they
         // simply keep the class from being abstract.
 
--- a/jdk/src/share/classes/javax/swing/text/View.java	Thu Sep 12 18:21:06 2013 +0400
+++ b/jdk/src/share/classes/javax/swing/text/View.java	Thu Sep 12 18:44:14 2013 +0400
@@ -1137,32 +1137,9 @@
      */
     protected void forwardUpdate(DocumentEvent.ElementChange ec,
                                       DocumentEvent e, Shape a, ViewFactory f) {
-        Element elem = getElement();
-        int pos = e.getOffset();
-        int index0 = getViewIndex(pos, Position.Bias.Forward);
-        if (index0 == -1 && e.getType() == DocumentEvent.EventType.REMOVE &&
-            pos >= getEndOffset()) {
-            // Event beyond our offsets. We may have represented this, that is
-            // the remove may have removed one of our child Elements that
-            // represented this, so, we should foward to last element.
-            index0 = getViewCount() - 1;
-        }
-        int index1 = index0;
-        View v = (index0 >= 0) ? getView(index0) : null;
-        if (v != null) {
-            if ((v.getStartOffset() == pos) && (pos > 0)) {
-                // If v is at a boundary, forward the event to the previous
-                // view too.
-                index0 = Math.max(index0 - 1, 0);
-            }
-        }
-        if (e.getType() != DocumentEvent.EventType.REMOVE) {
-            index1 = getViewIndex(pos + e.getLength(), Position.Bias.Forward);
-            if (index1 < 0) {
-                index1 = getViewCount() - 1;
-            }
-        }
-        int hole0 = index1 + 1;
+        calculateUpdateIndexes(e);
+
+        int hole0 = lastUpdateIndex + 1;
         int hole1 = hole0;
         Element[] addedElems = (ec != null) ? ec.getChildrenAdded() : null;
         if ((addedElems != null) && (addedElems.length > 0)) {
@@ -1173,11 +1150,9 @@
         // forward to any view not in the forwarding hole
         // formed by added elements (i.e. they will be updated
         // by initialization.
-        index0 = Math.max(index0, 0);
-        index1 = Math.max((getViewCount() - 1), 0);
-        for (int i = index0; i <= index1; i++) {
+        for (int i = firstUpdateIndex; i <= lastUpdateIndex; i++) {
             if (! ((i >= hole0) && (i <= hole1))) {
-                v = getView(i);
+                View v = getView(i);
                 if (v != null) {
                     Shape childAlloc = getChildAllocation(i, a);
                     forwardUpdateToView(v, e, childAlloc, f);
@@ -1187,6 +1162,39 @@
     }
 
     /**
+     * Calculates the first and the last indexes of the child views
+     * that need to be notified of the change to the model.
+     * @param e the change information from the associated document
+     */
+    void calculateUpdateIndexes(DocumentEvent e) {
+        int pos = e.getOffset();
+        firstUpdateIndex = getViewIndex(pos, Position.Bias.Forward);
+        if (firstUpdateIndex == -1 && e.getType() == DocumentEvent.EventType.REMOVE &&
+            pos >= getEndOffset()) {
+            // Event beyond our offsets. We may have represented this, that is
+            // the remove may have removed one of our child Elements that
+            // represented this, so, we should forward to last element.
+            firstUpdateIndex = getViewCount() - 1;
+        }
+        lastUpdateIndex = firstUpdateIndex;
+        View v = (firstUpdateIndex >= 0) ? getView(firstUpdateIndex) : null;
+        if (v != null) {
+            if ((v.getStartOffset() == pos) && (pos > 0)) {
+                // If v is at a boundary, forward the event to the previous
+                // view too.
+                firstUpdateIndex = Math.max(firstUpdateIndex - 1, 0);
+            }
+        }
+        if (e.getType() != DocumentEvent.EventType.REMOVE) {
+            lastUpdateIndex = getViewIndex(pos + e.getLength(), Position.Bias.Forward);
+            if (lastUpdateIndex < 0) {
+                lastUpdateIndex = getViewCount() - 1;
+            }
+        }
+        firstUpdateIndex = Math.max(firstUpdateIndex, 0);
+    }
+
+    /**
      * Forwards the <code>DocumentEvent</code> to the give child view.  This
      * simply messages the view with a call to <code>insertUpdate</code>,
      * <code>removeUpdate</code>, or <code>changedUpdate</code> depending
@@ -1345,4 +1353,14 @@
     private View parent;
     private Element elem;
 
+    /**
+     * The index of the first child view to be notified.
+     */
+    int firstUpdateIndex;
+
+    /**
+     * The index of the last child view to be notified.
+     */
+    int lastUpdateIndex;
+
 };
--- a/jdk/test/javax/swing/text/View/8014863/bug8014863.java	Thu Sep 12 18:21:06 2013 +0400
+++ b/jdk/test/javax/swing/text/View/8014863/bug8014863.java	Thu Sep 12 18:44:14 2013 +0400
@@ -24,6 +24,7 @@
 /*
  * @test
  * @bug 8014863
+ * @bug 8024395
  * @summary  Tests the calculation of the line breaks when a text is inserted
  * @author Dmitry Markov
  * @library ../../../regtesthelpers
@@ -34,91 +35,107 @@
 import sun.awt.SunToolkit;
 
 import javax.swing.*;
+import javax.swing.text.GlyphView;
+import javax.swing.text.View;
 import javax.swing.text.html.HTMLEditorKit;
 import java.awt.*;
 import java.awt.event.KeyEvent;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.Arrays;
 
 public class bug8014863 {
 
     private static JEditorPane editorPane;
+    private static JFrame frame;
     private static Robot robot;
     private static SunToolkit toolkit;
 
+    private static String text1 = "<p>one two qqqq <em>this is a test sentence</em> qqqq <em>pp</em> qqqq <em>pp</em> " +
+            "qqqq <em>pp</em> qqqq <em>pp</em> qqqq <em>pp</em> qqqq <em>pp</em> qqqq <em>pp</em> qqqq <em>pp</em> qqqq</p>";
+    private static String text2 = "<p>qqqq <em>this is a test sentence</em> qqqq <em>pp</em> qqqq <em>pp</em> " +
+            "qqqq <em>pp</em> qqqq <em>pp</em> qqqq <em>pp</em> qqqq <em>pp</em> qqqq <em>pp</em> qqqq <em>pp</em> qqqq</p>";
+
+    private static ArrayList<GlyphView> glyphViews;
+
     public static void main(String[] args) throws Exception {
         toolkit = (SunToolkit) Toolkit.getDefaultToolkit();
         robot = new Robot();
+        robot.setAutoDelay(50);
+        glyphViews = new ArrayList<GlyphView>();
 
-        createAndShowGUI();
+        createAndShowGUI(text1);
+
+        toolkit.realSync();
+
+        SwingUtilities.invokeAndWait(new Runnable() {
+            public void run() {
+                retrieveGlyphViews(editorPane.getUI().getRootView(editorPane));
+            }
+        });
+        GlyphView [] arr1 = glyphViews.toArray(new GlyphView[glyphViews.size()]);
+
+        frame.dispose();
+        glyphViews.clear();
+
+        createAndShowGUI(text2);
 
         toolkit.realSync();
 
         Util.hitKeys(robot, KeyEvent.VK_HOME);
-        Util.hitKeys(robot, KeyEvent.VK_O);
-
         toolkit.realSync();
 
-        if (3 != getNumberOfTextLines()) {
-            throw new RuntimeException("The number of texts lines does not meet the expectation");
-        }
-
+        Util.hitKeys(robot, KeyEvent.VK_O);
         Util.hitKeys(robot, KeyEvent.VK_N);
-
-        toolkit.realSync();
-
-        if (3 != getNumberOfTextLines()) {
-            throw new RuntimeException("The number of texts lines does not meet the expectation");
-        }
-
         Util.hitKeys(robot, KeyEvent.VK_E);
         Util.hitKeys(robot, KeyEvent.VK_SPACE);
         Util.hitKeys(robot, KeyEvent.VK_T);
         Util.hitKeys(robot, KeyEvent.VK_W);
+        Util.hitKeys(robot, KeyEvent.VK_O);
+        Util.hitKeys(robot, KeyEvent.VK_SPACE);
 
         toolkit.realSync();
 
-        if (3 != getNumberOfTextLines()) {
-            throw new RuntimeException("The number of texts lines does not meet the expectation");
+        SwingUtilities.invokeAndWait(new Runnable() {
+            public void run() {
+                retrieveGlyphViews(editorPane.getUI().getRootView(editorPane));
+            }
+        });
+        GlyphView [] arr2 = glyphViews.toArray(new GlyphView[glyphViews.size()]);
+
+        if (arr1.length != arr2.length) {
+            throw new RuntimeException("Test Failed!");
+        }
+
+        for (int i=0; i<arr1.length; i++) {
+            GlyphView v1 = arr1[i];
+            GlyphView v2 = arr2[i];
+            Field field = GlyphView.class.getDeclaredField("breakSpots");
+            field.setAccessible(true);
+            int[] breakSpots1 = (int[])field.get(v1);
+            int[] breakSpots2 = (int[])field.get(v2);
+            if (!Arrays.equals(breakSpots1,breakSpots2)) {
+                throw new RuntimeException("Test Failed!");
+            }
+        }
+
+        frame.dispose();
+    }
+
+    private static void retrieveGlyphViews(View root) {
+        for (int i=0; i<= root.getViewCount()-1; i++) {
+            View view = root.getView(i);
+            if (view instanceof GlyphView && view.isVisible()) {
+                if (!glyphViews.contains(view)) {
+                    glyphViews.add((GlyphView)view);
+                }
+            } else {
+                retrieveGlyphViews(view);
+            }
         }
     }
 
-    private static int getNumberOfTextLines() throws Exception {
-        int numberOfLines = 0;
-        int caretPosition = getCaretPosition();
-        int current = 1;
-        int previous;
-
-        setCaretPosition(current);
-        do {
-            previous = current;
-            Util.hitKeys(robot, KeyEvent.VK_DOWN);
-            toolkit.realSync();
-            current = getCaretPosition();
-            numberOfLines++;
-        } while (current != previous);
-
-        setCaretPosition(caretPosition);
-        return numberOfLines;
-    }
-
-    private static int getCaretPosition() throws Exception {
-        final int[] result = new int[1];
-        SwingUtilities.invokeAndWait(new Runnable() {
-            public void run() {
-                result[0] = editorPane.getCaretPosition();
-            }
-        });
-        return result[0];
-    }
-
-    private static void setCaretPosition(final int position) throws Exception {
-        SwingUtilities.invokeAndWait(new Runnable() {
-            public void run() {
-                editorPane.setCaretPosition(position);
-            }
-        });
-    }
-
-    private static void createAndShowGUI() throws Exception {
+    private static void createAndShowGUI(String text) throws Exception {
         SwingUtilities.invokeAndWait(new Runnable() {
             public void run() {
                 try {
@@ -126,22 +143,17 @@
                 } catch (Exception ex) {
                     throw new RuntimeException(ex);
                 }
-                JFrame frame = new JFrame();
+                frame = new JFrame();
                 frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
 
                 editorPane = new JEditorPane();
                 HTMLEditorKit editorKit = new HTMLEditorKit();
                 editorPane.setEditorKit(editorKit);
-                editorPane.setText("<p>qqqq <em>pp</em> qqqq <em>pp</em> " +
-                        "qqqq <em>pp</em> qqqq <em>pp</em> qqqq <em>pp</em> qqqq <em>pp" +
-                        "</em> qqqq <em>pp</em> qqqq <em>pp</em> qqqq <em>pp</em> qqqq</p>");
+                editorPane.setText(text);
                 editorPane.setCaretPosition(1);
-                // An actual font size depends on OS and might be differnet on various OSs.
-                // It is necessary to calculate the width to meet the expected number of lines.
-                int width = SwingUtilities.computeStringWidth(editorPane.getFontMetrics(editorPane.getFont()),
-                        "qqqq pp qqqq pp qqqq pp qqqqqqqq");
+
                 frame.add(editorPane);
-                frame.setSize(width, 200);
+                frame.setSize(200, 200);
                 frame.setVisible(true);
             }
         });