JDK-8095471 : [TableView] TableCell border lines are rendered incorrectly
  • Type: Bug
  • Component: javafx
  • Sub-Component: graphics
  • Affected Version: 8u40
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2014-12-05
  • Updated: 2015-06-12
  • Resolved: 2014-12-06
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
8u40Fixed
Related Reports
Relates :  
Relates :  
Relates :  
Description
Run this sample program 
"

import javafx.application.Application;
import javafx.beans.property.SimpleStringProperty;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.event.EventHandler;
import javafx.scene.Group;
import javafx.scene.Scene;
import javafx.scene.control.ContextMenu;
import javafx.scene.control.Label;
import javafx.scene.control.MenuItem;
import javafx.scene.control.SelectionMode;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableColumn.CellEditEvent;
import javafx.scene.control.TableView;
import javafx.scene.control.cell.PropertyValueFactory;
import javafx.scene.control.cell.TextFieldTableCell;
import javafx.scene.layout.HBox;
import javafx.scene.text.Font;
import javafx.stage.Stage;

public class TableViewSample extends Application {

    private TableView<Person> table = new TableView<Person>();
    private final ObservableList<Person> data
            = FXCollections.observableArrayList();
    final HBox hb = new HBox();

    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage stage) {
        Scene scene = new Scene(new Group());
        scene.getStylesheets().add("test.css");
        stage.setTitle("Table View Sample");
        stage.setWidth(450);
        stage.setHeight(550);

        table.setMaxWidth(450);
        for (int i = 0; i < 10; ++i) {
            data.add(new Person(String.valueOf(i), "lol", "lol"));
        }

        final Label label = new Label("Address Book");
        label.setFont(new Font("Arial", 20));

        ContextMenu menu = new ContextMenu();
        MenuItem item = new MenuItem("test");
        menu.getItems().add(item);
        table.setContextMenu(menu);
        table.setEditable(false);
        table.getSelectionModel().setCellSelectionEnabled(true);
        table.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);

        table.setItems(data);
        for (int i = 0; i < 10; ++i) {
            TableColumn emailCol = new TableColumn("Email");
            emailCol.setMinWidth(200);
            emailCol.setCellValueFactory(
                    new PropertyValueFactory<Person, String>("email"));
            emailCol.setCellFactory(TextFieldTableCell.forTableColumn());
            emailCol.setOnEditCommit(
                    new EventHandler<CellEditEvent<Person, String>>() {
                        @Override
                        public void handle(CellEditEvent<Person, String> t) {
                            ((Person) t.getTableView().getItems().get(
                                    t.getTablePosition().getRow())).setEmail(t.getNewValue());
                        }
                    }
            );
            table.getColumns().add(emailCol);
        }


        ((Group) scene.getRoot()).getChildren().addAll(table);

        stage.setScene(scene);
        stage.show();
    }

    public static class Person {

        private final SimpleStringProperty firstName;
        private final SimpleStringProperty lastName;
        private final SimpleStringProperty email;

        private Person(String fName, String lName, String email) {
            this.firstName = new SimpleStringProperty(fName);
            this.lastName = new SimpleStringProperty(lName);
            this.email = new SimpleStringProperty(email);
        }

        public String getFirstName() {
            return firstName.get();
        }

        public void setFirstName(String fName) {
            firstName.set(fName);
        }

        public String getLastName() {
            return lastName.get();
        }

        public void setLastName(String fName) {
            lastName.set(fName);
        }

        public String getEmail() {
            return email.get();
        }

        public void setEmail(String fName) {
            email.set(fName);
        }
    }
}
"

You will need a "test.css" file with that inside 

"

.table-cell{
    -fx-padding: 0 0 0 0.2em;
    -fx-border-color: black;
    -fx-border-width : 0.3px;
    -fx-background-color: -fx-table-cell-border-color,white;
}
"

The border of the TableCell should be black but instead you can see that the borders only appear when you click on them. When you drag, all borders are gone except the columns edge when keep dragging..

This is a serious regression that appears on JDK 8u40b16 and was not there on JDK8u40 b15.
Comments
I am ok with the change. I will keep a closer eye on these controls in my sanity testing.
07-12-2014

Interesting. In debugging the dirtyopts issue I discovered that the AA rects aren't being rendered from any of the rectangles that I just fixed the smooth flag from. They are actually being rendered from NGRegion - which does not have a smooth setting and is inheriting the flag from NGShape which modifies the AA flag in the graphics, but never restores it. I have a patch forthcoming which will fix NGShape to restore the flag properly as soon as I can create a proper test case for it. So, the calls to setSmooth() that I identified above were not the direct cause, but caused this problem as a side effect. I think we can still cause the problem if we get a particular combination of smooth shape nodes and regions mixed in the SG... The fact that they were alternating between an AA border and a solid border was actually due to the way that dirty regions avoid rendering the nodes that were setting the flag in the graphics. With a full repaint, the flag would be set for the Region rendering. With a partial repaint of just some of the SG due to dirtyopts, the flag would not be set and be left at the default AA=true setting and we would see old AA borders being left behind. I still think the indicated rectangles shouldn't be AA, but it turns out that I don't think we needed to fix them in order to fix this bug.
06-12-2014

Works for me too.
06-12-2014

Noting that Kevin and I are still awaiting a post-commit review from Jonathan, even though the issue is marked as Resolved now...
06-12-2014

Fixed in the 8u-dev repo with the following changeset: changeset: 8469:8bd5a0d0c97b date: Fri Dec 05 16:50:07 2014 -0800 summary: Fix RT-39590: TableCell border lines are rendered incorrectly http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/8bd5a0d0c97b
06-12-2014

After chatting with Kevin we decided that in order to make maximal use of our sanity testing on Monday that I would push this fix now and if Jonathan has any objections we can file a new Jira. We were also both fairly confident that the right thing to do with TextInputControlSkin was to just remove the setSmooth() call so I deleted that entirely (and removed the comment) before pushing. Here is the updated webrev for the final fix that I will push: http://cr.openjdk.java.net/~flar/RT-39590/webrev.01/
06-12-2014

The fix looks good to me. I will defer to Jonathan on the question of whether or not to remove the call from TextInputControlSkin (I also expect that removing it is correct, but Jonathan can make that call). +1
06-12-2014

I also grepped for the -fx-smooth attribute in all of our internal css files and didn't find any references, but that is another way that we could be setting up non-smooth shapes which may not be wise given that they've never been non-smooth before recent changes (except for clipping where non-smooth rectangles are an optimization that we have recognized since the first release).
06-12-2014

In looking at how the "attrs" is used in TextInputControlSkin, I am 95% sure that these nodes will be used for rendering instead of clipping so I think the right answer there is to delete the call, but I'd like to hear from Jonathan first to be sure...
06-12-2014

Here are my proposed fixes (removals) for the unnecessary setSmooth(false) calls: http://cr.openjdk.java.net/~flar/RT-39590/webrev.00/ Note that I wasn't sure about that last file (TextInputControlSkin) and simply commented out the line with a question in the code comment. Depending on Jonathan's assessment/answer I could either just delete it like the rest or restore it - let me know which way I should go on that one.
06-12-2014

Here are the list of calls to setSmooth() in the rt repo. The 3 that I think are suspect are the ones in ColorPalette, NestedTableColumnHeader (the one triggered by this test case), and TextInputControlSkin. Usages of Shape.setSmooth [9 occurrences] controls ColorPalette.java ���� 441:����rectangle.setSmooth(false); ContextMenuContent.java ���� 109:����clipRect.setSmooth(false); NestedTableColumnHeader.java ���� 532:����rect.setSmooth(false); TableHeaderRow.java ���� 156:����clip.setSmooth(false); TextFieldSkin.java ���� 163:����clip.setSmooth(false); TextInputControlSkin.java ���� 604:����attr.setSmooth(false); VirtualFlow.java 2665:����clipRect.setSmooth(false); XYChart.java ���� 449:����plotAreaClip.setSmooth(false); graphics ShapeTest.java ���� 105:����shape.setSmooth(true);
05-12-2014

A number of rectangles used for rendering in the controls have their smooth property set to false - including these table borders. There are a few calls to rectangle.setSmooth() being used on Rectangle nodes that will be set as another Node's clip - and those make sense because it establishes a scissor clip that can be optimized in the rendering pipeline, but the ones that are added to groups to be rendered should probably be left antialiased and may have been set to smooth(false) as a result of cutting and pasting some code that was used for clipping.
05-12-2014

Here is the screenshot of the secondary (dirtyopts) bug: Table-dirty.png -- screenshot after clicking on the following cells in order: (0,0), (2,0), (4,0), (6,1)
05-12-2014

I tried to generate an image of the secondary problem, but as soon as I go to generate the screenshot of the window, the app loses focus and repairs itself. I will modify the program to put in a sleep and then try to capture an image.
05-12-2014

Attached two images: table-good-b15.png -- good image generated with 8u40-b15 table-bad.png -- bad image generated from the current 8u-dev repo
05-12-2014

I will note that it is quite possible that the fix for RT-39424 merely exposed a bug elsewhere (a bad assumption), but either way, this is a serious regression that we must fix.
05-12-2014

This is a graphics bug. I tracked the disappearing borders down to the following changeset: changeset: 8424:d094365ab0c5 tag: 8u40-b16 user: flar <James.Graham@oracle.com> date: Sun Nov 23 21:19:59 2014 -0800 summary: Fix RT-39424: non-AA rendering should sample at centers of pixels This was a follow-on fix for: changeset: 8419:438298641586 user: Chien Yang <chien.yang@oracle.com> date: Fri Nov 21 14:08:34 2014 -0800 summary: Fix to RT-39468: Overlapping 3D transformed 2D shapes have dirty edges in Scene with depth buffer I also notice that the fix for RT-39468 introduced a secondary bug (apparently related to dirty region calculations). If I revert just RT-39424, then I notice that the grid of cells is initially drawn correctly, but if you click on a cell, which causes a slightly thicker line to be drawn, it isn't quite rendered correctly; this is most noticeable when you click on another cell -- the first cell is not redrawn correctly (it should be rendered as it was initially). A window resize or refresh fixes this secondary problem. So does running it with -Dprism.dirtyopts=false. However, turning dirtyopts off has no impact on the primary bug, other than to make it consistently drawn with no border (with dirtyopts enabled, you get "leftover" borders when clicking on a cell and clicking on a different cell).
05-12-2014