Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#9634 closed defect (fixed)

Downloading Referrers Grabs Focus

Reported by: ingalls Owned by: team
Priority: normal Milestone: 14.04
Component: Core remotecontrol Version: tested
Keywords: Cc:

Description

When Downloading nodes through the remote, JOSM grabs focus each time a 'downloading referrs' dialogue appears. Since there is a new dialogue for each and every node being downloaded, large remote downloads make the host computer unusable as JOSM is constantly grabbing focus.

Attachments (4)

ticket9634_2014-04-06_08:56:16_+0200.patch (13.6 KB) - added by Balaitous 6 years ago.
Draft of current work
ticket9634_2014-04-10_00:46:09_+0200.patch (20.3 KB) - added by Balaitous 6 years ago.
Draft of current work
ticket9634_2014-04-11_00:53:42_+0200.patch (23.7 KB) - added by Balaitous 6 years ago.
Draft of current work, I have found where downloaded primitives are selected. In a good way.
ticket9634_2014-04-12_00:53:36_+0200.patch (23.5 KB) - added by Balaitous 6 years ago.
I think this time it is OK

Download all attachments as: .zip

Change History (18)

comment:1 Changed 6 years ago by Don-vip

Milestone: 14.02
Priority: majornormal

Request example:

http://localhost:8111/load_object?objects=n357622684,n357623764,n2493782437,n2493782445,n2493782455,n2493782475,n2493782517,n2493782529,n2493782532,n2493782536,n2493782559,n2493782562,n2493782610,n2493782616,n2493782625,n2493782632,n2493782646,n2493782661,n2493863533,n2493863668,n2493964455,n2493964594,n2493964947,n2493964952,n2493983896,n2493983907,n2493983922,n2493983970,n2493983979,n2493983984,n2493984002,n2493984008,n2493984036,n2493984054,n2493984212,n2493984268,n2493984282,n2494002191,n2494002218,n2494002229,n2494002236,n2494002239,n2494002249,n2494002251,n2494002266,n2494002308,n2494002313,n2494002317,n2494002331,n2494002337,n2494002376,n2494002384,n2494002654,n2494002769,n2494002794,n2494002851,n2494018287,n2494018288,n2494018292,n2494018299,n2494018302,n2494018304,n2494018314,n2494018331,n2494018368,n2494018387,n2494018390,n2494018401,n2494018410,n2494018419,n2494023442,n2494023445,n2494023704,n2494023817,n2494023866,n2494027649,n2494027652,n2494027654,n2494027672,n2494027675,n2494033294,n2494041724,n2494045819,n2494045824,n2494045850,n2494045899,n2494045951,n2494046073,n2494046075,n2494046116,n2494046272,n2494046283,n2494046290,n2494046324,n2494046330,n2494046333,n2494046335,n2494046458

comment:2 Changed 6 years ago by Don-vip

Milestone: 14.0214.03

comment:3 Changed 6 years ago by Don-vip

Milestone: 14.0314.04

comment:4 Changed 6 years ago by Balaitous

I am trying to do something. But I am not friendly with thread, and I don't understand every thing.
One of the problem is how to do Cancel.
It's rather easy to do a Stop button (stop downloading next referrers), but a real Cancel button need to remove all downloading object !

I put code here since in diff file it's not easy to see (it's mainly the older code, but reoder)
I have put some FIXME comment where I have problems.

    /**
     * @param newLayer if the data should be downloaded into a new layer
     * @param ids
     * @param downloadReferrers if the referrers of the object should be downloaded as well, i.e., parent relations, and for nodes, additionally, parent ways
     * @param full if the members of a relation should be downloaded as well
     */
    public static void processItems(boolean newLayer, final List<PrimitiveId> ids, boolean downloadReferrers, boolean full) {
 
        class DownloadPrimitivesWithReferrersTask extends PleaseWaitRunnable {
            final OsmDataLayer layer;
            List<PrimitiveId> ids;
            boolean full;
            DownloadPrimitivesTask task;
            
            DownloadPrimitivesWithReferrersTask(final OsmDataLayer layer, List<PrimitiveId> ids,
                    boolean full) { //, ProgressMonitor monitor) {
                super(tr("Download objects"), false);
                this.layer = layer;
                this.ids = ids;
                this.full = full;
            }
            
            @Override
            protected void cancel() {
                // FIXME: how to do ?
            }

            @Override
            protected void realRun() throws SAXException, IOException, OsmTransferException {
                task = new DownloadPrimitivesTask(layer, ids, full, getProgressMonitor().createSubTaskMonitor(1, false));
                // FIXME: Need to run task in an other thread (like Main.worker.submit(task) ?
                task.run();
                getProgressMonitor().setTicksCount(ids.size());
                for(PrimitiveId id : ids) {
                    System.out.printf("referrers task %s\n", id.toString());
                    // FIXME: In an other thread ?
                    new DownloadReferrersTask(layer, id, getProgressMonitor().createSubTaskMonitor(1, false)).run();
                }
            }

            @Override
            protected void finish() {
                // FIXME: Does all task are really finish ?
                System.out.printf("call to finish\n");
                Runnable showErrorsAndWarnings = new Runnable() {
                    @Override
                    public void run() {
                        final Set<PrimitiveId> errs = task.getMissingPrimitives();
                        if (errs != null && !errs.isEmpty()) {
                            try {
                                SwingUtilities.invokeAndWait(new Runnable() {
                                    @Override
                                    public void run() {
                                        reportProblemDialog(errs,
                                                trn("Object could not be downloaded", "Some objects could not be downloaded", errs.size()),
                                                trn("One object could not be downloaded.<br>",
                                                    "{0} objects could not be downloaded.<br>",
                                                    errs.size(),
                                                    errs.size())
                                                + tr("The server replied with response code 404.<br>"
                                                    + "This usually means, the server does not know an object with the requested id."),
                                                tr("missing objects:"),
                                                JOptionPane.ERROR_MESSAGE
                                        ).showDialog();
                                    }
                                });
                            } catch (InterruptedException ex) {
                                Main.warn("InterruptedException while displaying error dialog");
                            } catch (InvocationTargetException ex) {
                                Main.warn(ex);
                            }
                        }

                        final Set<PrimitiveId> del = new TreeSet<PrimitiveId>();
                        DataSet ds = getCurrentDataSet();
                        for (PrimitiveId id : ids) {
                            OsmPrimitive osm = ds.getPrimitiveById(id);
                            if (osm != null && osm.isDeleted()) {
                                del.add(id);
                            }
                        }
                        if (!del.isEmpty()) {
                            SwingUtilities.invokeLater(new Runnable() {
                                @Override
                                public void run() {
                                    reportProblemDialog(del,
                                            trn("Object deleted", "Objects deleted", del.size()),
                                            trn(
                                                "One downloaded object is deleted.",
                                                "{0} downloaded objects are deleted.",
                                                del.size(),
                                                del.size()),
                                            null,
                                            JOptionPane.WARNING_MESSAGE
                                    ).showDialog();
                                }
                            });
                        }
                    }
                };
                Main.worker.submit(showErrorsAndWarnings);
            }
        }
        
        System.out.printf("DEBUG: DownloadPrimitiveAction.processItems\n");
        OsmDataLayer layer = getEditLayer();
        if ((layer == null) || newLayer) {
            layer = new OsmDataLayer(new DataSet(), OsmDataLayer.createNewName(), null);
            Main.main.addLayer(layer);
        }

        final DownloadPrimitivesWithReferrersTask task = 
                new DownloadPrimitivesWithReferrersTask(layer, ids, full); //, monitor);
        Main.worker.submit(task);

        System.out.printf("task done\n");
    }

Changed 6 years ago by Balaitous

Draft of current work

comment:5 in reply to:  4 Changed 6 years ago by bastiK

Replying to Balaitous:

I am trying to do something. But I am not friendly with thread, and I don't understand every thing.
One of the problem is how to do Cancel.
It's rather easy to do a Stop button (stop downloading next referrers), but a real Cancel button need to remove all downloading object !

I put code here since in diff file it's not easy to see (it's mainly the older code, but reoder)
I have put some FIXME comment where I have problems.

Hi Balaitous,

Looks good so far! For cancel, you could save the DownloadPrimitivesTask and the DownloadReferrersTasks in fields, so they can be accessed from the cancel method. Then do something like

            @Override
            protected void cancel() {
                if (task != null) {
                    task.cancel();
                }
                // ...
            }

I think it is correct to run the tasks directly as you do, without any additional parallel processing. This ensures that they are indeed finished, when finish() is invoked.

Make sure you don't forget the check for the downloadReferrers flag. Also you may want to check if the handling of the ProgressMonitor can be improved. You may have to play a little bit with the options and look at examples, the code for progress update is not too well documented.

Changed 6 years ago by Balaitous

Draft of current work

comment:6 Changed 6 years ago by Balaitous

I have try to make download into a temporary layer, but a lot of problems remains.
Maybe it should be better to adapt DownloadPrimitivesTask class be setting a downloadReferrers flag, and work with dataset instead of layer.

comment:7 in reply to:  6 Changed 6 years ago by bastiK

Replying to Balaitous:

I have try to make download into a temporary layer, but a lot of problems remains.

What kind of problems?

comment:8 Changed 6 years ago by Balaitous

In order to be able to do a full cancel, all primitives and referrers have to be downloaded into a tmpLayer that is not attach to any mapFrame.
This involved that it is possible to call DownloadPrimitivesTask and DownloadReferrersTask before Main.map init.
So I have had test if(Main.map != null).
Another problem is that when action is cancelled, primitives from the tmpLayer remains selected.

Changed 6 years ago by Balaitous

Draft of current work, I have found where downloaded primitives are selected. In a good way.

Changed 6 years ago by Balaitous

I think this time it is OK

comment:9 Changed 6 years ago by bastiK

Resolution: fixed
Status: newclosed

In 6973/josm:

applied #9634 - Downloading Referrers Grabs Focus (patch by Balaitous, minor modifications)

comment:10 Changed 6 years ago by bastiK

Great work! I did minor changes to avoid EDT violation.

comment:11 Changed 6 years ago by Balaitous

I don't understand every thing with thread. I have a little experience with MPI and openMP, but nothing with java!

comment:12 Changed 6 years ago by bastiK

Then you probably know more about threads than me. Just take care that GUI stuff (like changing the selection) is done in event dispatch thread (EDT). (And everything that takes a long time, like download, must not be done in EDT, otherwise the program will freeze.) I only noticed because there were some warnings on the command line.

Last edited 6 years ago by bastiK (previous) (diff)

comment:13 Changed 6 years ago by Don-vip

EDT violations are easy to spot. Just look at warnings printed on Console when you run the program from a local build (i.e Eclipse) :)

comment:14 Changed 6 years ago by Don-vip

In 6980/josm:

see #9634 - fix sonar issue - inconsistent synchronization

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.