Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NETBEANS-977] Improve text layout in word wrap mode. #598

Merged
merged 1 commit into from Sep 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
109 changes: 64 additions & 45 deletions ide/editor.lib2/src/org/netbeans/api/editor/caret/EditorCaret.java
Expand Up @@ -25,7 +25,6 @@
import java.awt.Color;
import java.awt.Component;
import java.awt.Composite;
import java.awt.Container;
import java.awt.Cursor;
import java.awt.Dimension;
import java.awt.Graphics;
Expand Down Expand Up @@ -71,7 +70,6 @@
import javax.swing.SwingUtilities;
import javax.swing.Timer;
import javax.swing.TransferHandler;
import javax.swing.UIManager;
import javax.swing.event.ChangeEvent;
import javax.swing.event.ChangeListener;
import javax.swing.event.DocumentEvent;
Expand All @@ -87,7 +85,6 @@
import javax.swing.text.NavigationFilter;
import javax.swing.text.Position;
import javax.swing.text.StyleConstants;
import javax.swing.text.Utilities;
import org.netbeans.api.annotations.common.CheckForNull;
import org.netbeans.api.annotations.common.NonNull;
import org.netbeans.api.annotations.common.NullAllowed;
Expand Down Expand Up @@ -1028,21 +1025,17 @@ public void deinstall(JTextComponent c) {
* For that, the nearest update must reposition scroller's viewrect so that the
* recomputed caretBounds is at the same
*/
private synchronized void maybeSaveCaretOffset(JTextComponent c, Rectangle cbounds) {
if (c.getClientProperty("editorcaret.updateRetainsVisibleOnce") == null || // NOI18N
private synchronized void maybeSaveCaretOffset(Rectangle cbounds) {
if (component.getClientProperty("editorcaret.updateRetainsVisibleOnce") == null || // NOI18N
lastCaretVisualOffset != -1) {
return;
}
Component parent = c.getParent();
Rectangle editorRect;
if (parent instanceof JLayeredPane) {
parent = parent.getParent();
}
if (parent instanceof JViewport) {
final JViewport viewport = (JViewport) parent;
final JViewport viewport = getViewport();
if (viewport != null) {
editorRect = viewport.getViewRect();
} else {
Dimension size = c.getSize();
Dimension size = component.getSize();
editorRect = new Rectangle(0, 0, size.width, size.height);
}
if (cbounds.y >= editorRect.y && cbounds.y < (editorRect.y + editorRect.height)) {
Expand All @@ -1059,7 +1052,7 @@ private synchronized void maybeSaveCaretOffset(JTextComponent c, Rectangle cboun

@Override
public void paint(Graphics g) {
JTextComponent c = component;
final JTextComponent c = component;
if (c == null || !isShowing()) {
return;
}
Expand Down Expand Up @@ -1109,7 +1102,7 @@ public void paint(Graphics g) {
Rectangle newCaretBounds = lvh.modelToViewBounds(dot, Position.Bias.Forward);
Rectangle oldBounds = caretItem.setCaretBoundsWithRepaint(newCaretBounds, c, "EditorCaret.paint()", i);
if (caretItem == lastCaret && oldBounds != null) {
maybeSaveCaretOffset(c, oldBounds);
maybeSaveCaretOffset(oldBounds);
}
}
Rectangle caretBounds = caretItem.getCaretBounds();
Expand Down Expand Up @@ -1956,6 +1949,26 @@ private void dispatchUpdate(boolean forceInvokeLater) {
*/
private int lastCaretVisualOffset = -1;

private boolean isWrapping() {
// See o.n.modules.editor.lib2.view.DocumentViewOp.updateLineWrapType().
Object lwt = null;
if (component != null) {
lwt = component.getClientProperty(SimpleValueNames.TEXT_LINE_WRAP);
if (lwt == null) {
lwt = component.getDocument().getProperty(SimpleValueNames.TEXT_LINE_WRAP);
}
}
return (lwt instanceof String) && !"none".equals(lwt);
}

private JViewport getViewport() {
Component parent = component.getParent();
if (parent instanceof JLayeredPane) {
parent = parent.getParent();
}
return (parent instanceof JViewport) ? (JViewport) parent : null;
}

/**
* Update the caret's visual position.
* <br>
Expand All @@ -1973,13 +1986,9 @@ private void update(boolean calledFromPaint) {
if (c != null) {
boolean forceUpdate = c.getClientProperty("editorcaret.updateRetainsVisibleOnce") != null;
boolean log = LOG.isLoggable(Level.FINE);
Component parent = c.getParent();
Rectangle editorRect;
if (parent instanceof JLayeredPane) {
parent = parent.getParent();
}
if (parent instanceof JViewport) {
final JViewport viewport = (JViewport) parent;
final JViewport viewport = getViewport();
if (viewport != null) {
editorRect = viewport.getViewRect();
} else {
Dimension size = c.getSize();
Expand All @@ -1989,7 +1998,7 @@ private void update(boolean calledFromPaint) {
Rectangle cbounds = getLastCaretItem().getCaretBounds();
if (cbounds != null) {
// save relative position of the main caret
maybeSaveCaretOffset(c, cbounds);
maybeSaveCaretOffset(cbounds);
}
}
if (!calledFromPaint && !c.isValid() /* && maintainVisible == null */) {
Expand Down Expand Up @@ -2028,27 +2037,40 @@ private void update(boolean calledFromPaint) {
}
if (caretBounds != null) {
Rectangle scrollBounds = new Rectangle(caretBounds); // Must possibly be cloned upon change
if (viewport != null && isWrapping()) {
/* When wrapping, only scroll to the right if the caret is
decisively outside the wrapped area (e.g. on a very long unbreakable
word). Otherwise, always scroll back to the left. When typing such
that the caret goes from the end of one wrap line to the next, the
new caret position might be one or more characters away from the
first character on the wrap line, so a regular
scroll-to-make-the-caret-visible would not do the job. */
if (scrollBounds.x <= viewport.getExtentSize().width) {
scrollBounds.x = 0;
scrollBounds.width = 1;
/* Avoid generating a drag-select as a result of the viewport
being automatically scrolled back to x=0 as a result of the user
clicking once to move the caret. */
if (viewport.getViewPosition().x > 0 && getDot() == getMark()) {
mouseState = MouseState.DEFAULT;
}
}
}
// Only scroll the view for the LAST caret to be visible
// For null old bounds (likely at begining of component displayment) ensure that a possible
// horizontal scrollbar would not hide the caret so enlarge the scroll bounds by hscrollbar height.
if (oldCaretBounds == null) {
Component viewport = c.getParent();
if (viewport instanceof JLayeredPane) {
viewport = viewport.getParent();
}
if (viewport instanceof JViewport) {
Component scrollPane = viewport.getParent();
if (scrollPane instanceof JScrollPane) {
JScrollBar hScrollBar = ((JScrollPane) scrollPane).getHorizontalScrollBar();
if (hScrollBar != null) {
int hScrollBarHeight = hScrollBar.getPreferredSize().height;
Dimension extentSize = ((JViewport) viewport).getExtentSize();
// If the extent size is high enough then extend
// the scroll region by extra vertical space
if (extentSize.height >= caretBounds.height + hScrollBarHeight) {
scrollBounds = new Rectangle(scrollBounds); // Clone
scrollBounds.height += hScrollBarHeight;
}
if (oldCaretBounds == null && viewport != null) {
Component scrollPane = viewport.getParent();
if (scrollPane instanceof JScrollPane) {
JScrollBar hScrollBar = ((JScrollPane) scrollPane).getHorizontalScrollBar();
if (hScrollBar != null) {
int hScrollBarHeight = hScrollBar.getPreferredSize().height;
Dimension extentSize = ((JViewport) viewport).getExtentSize();
// If the extent size is high enough then extend
// the scroll region by extra vertical space
if (extentSize.height >= caretBounds.height + hScrollBarHeight) {
scrollBounds = new Rectangle(scrollBounds); // Clone
scrollBounds.height += hScrollBarHeight;
}
}
}
Expand Down Expand Up @@ -2586,12 +2608,9 @@ public void actionPerformed(ActionEvent e) { // Blinker timer fired
// and if it's fired the caret's bounds will be checked whether
// they intersect with the horizontal scrollbar
// and if so the view will be scrolled.
Container parent = component.getParent();
if(parent instanceof JLayeredPane) {
parent = parent.getParent();
}
if (parent instanceof JViewport) {
parent = parent.getParent(); // parent of viewport
final JViewport viewport = getViewport();
if (viewport != null) {
Component parent = viewport.getParent();
if (parent instanceof JScrollPane) {
JScrollPane scrollPane = (JScrollPane) parent;
JScrollBar hScrollBar = scrollPane.getHorizontalScrollBar();
Expand Down
@@ -0,0 +1,107 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.netbeans.modules.editor.lib2.view;

import java.text.CharacterIterator;

/* This class was written from scratch. I considered using org.apache.pivot.text.CharacterIterator
from the Apache Pivot project, but it had bugs in the next() and previous() methods (trying to use
CharacterIterator.DONE = 65535 as an index). */
/**
* Adapter class for providing a {@link CharacterIterator} over a {@link CharSequence} without
* making a copy of the entire underlying string.
*
* @author Eirik Bakke (ebakke@ultorg.com)
*/
class CharSequenceCharacterIterator implements CharacterIterator {
private final CharSequence charSequence;
private int index;

public CharSequenceCharacterIterator(CharSequence charSequence) {
if (charSequence == null) {
throw new NullPointerException();
}
this.charSequence = charSequence;
}

@Override
public int getIndex() {
return index;
}

@Override
public char setIndex(int position) {
if (position < getBeginIndex() || position > getEndIndex()) {
throw new IllegalArgumentException();
}
this.index = position;
return current();
}

@Override
public char current() {
return index == getEndIndex() ? CharacterIterator.DONE : charSequence.charAt(index);
}

@Override
public char first() {
return setIndex(getBeginIndex());
}

@Override
public char last() {
final int endIndex = getEndIndex();
return setIndex(charSequence.length() == 0 ? endIndex : (endIndex - 1));
}

@Override
public char next() {
if (index < getEndIndex()) {
index++;
}
return current();
}

@Override
public char previous() {
if (index > getBeginIndex()) {
index--;
return current();
} else {
return CharacterIterator.DONE;
}
}

@Override
public int getBeginIndex() {
return 0;
}

@Override
public int getEndIndex() {
return charSequence.length();
}

@Override
public Object clone() {
CharacterIterator ret = new CharSequenceCharacterIterator(charSequence);
ret.setIndex(index);
return ret;
}
}
Expand Up @@ -1227,10 +1227,12 @@ float getAvailableWidth() {
setStatusBits(AVAILABLE_WIDTH_VALID);
availableWidth = Integer.MAX_VALUE;
renderWrapWidth = availableWidth;
TextLayout lineContTextLayout = getLineContinuationCharTextLayout();
if (lineContTextLayout != null && (getLineWrapType() != LineWrapType.NONE)) {
availableWidth = Math.max(getVisibleRect().width, 4 * getDefaultCharWidth() + lineContTextLayout.getAdvance());
renderWrapWidth = availableWidth - lineContTextLayout.getAdvance();
if (getLineWrapType() != LineWrapType.NONE) {
final TextLayout lineContTextLayout = getLineContinuationCharTextLayout();
final float lineContTextLayoutAdvance =
lineContTextLayout == null ? 0f : lineContTextLayout.getAdvance();
availableWidth = Math.max(getVisibleRect().width, 4 * getDefaultCharWidth() + lineContTextLayoutAdvance);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

original code did not change availableWidth if lineContTextLayout == null; is it OK to alter it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes; the getLineWrapType() != LineWrapType.NONE check is sufficient. Previously, getLineContinuationCharTextLayout would never return null in practice--only if the font could not display the line continuation character or its alternate. And if that ever happened, there would have been an NPE when WrapInfo calls paintTextLayout with the null layout.

I changed this so that getLineContinuationCharTextLayout can now return null without error (and added Javadoc to that effect). Rather than disabling line wrapping altogether, this simply avoids displaying displaying the line continuation character. Then I made this the default, unless the user opts to display hidden characters (paragraph marks etc).

I just looked over the call sites of getLineContinuationCharTextLayout again; they seem to do the right thing.

renderWrapWidth = availableWidth - lineContTextLayoutAdvance;
}
}
return availableWidth;
Expand Down Expand Up @@ -1343,7 +1345,17 @@ TextLayout getTabCharTextLayout(double availableWidth) {
return ret;
}

/**
* @return will be null if the line continuation character should not be shown
*/
TextLayout getLineContinuationCharTextLayout() {
/* The line continuation character is used to show that a line is automatically being
broken into multiple wrap lines via the line wrap feature. This causes a lot of visual
clutter, and always takes up an extra character of horizontal space, so don't show it by
default. The same information is communicated by the line numbers in the left-hand side of
the editor anyway. */
if (!docView.op.isNonPrintableCharactersVisible())
return null;
if (lineContinuationTextLayout == null) {
char lineContinuationChar = LINE_CONTINUATION;
if (!defaultFont.canDisplay(lineContinuationChar)) {
Expand Down