diff --git a/untranslatable_pages/intro_hacking_krita.rst b/untranslatable_pages/intro_hacking_krita.rst index 67394968e..091e5279b 100644 --- a/untranslatable_pages/intro_hacking_krita.rst +++ b/untranslatable_pages/intro_hacking_krita.rst @@ -1,163 +1,165 @@ .. meta:: :description: Introduction to hacking Krita. .. metadata-placeholder :authors: - Michael Abrahams - Wolthera van Hövell tot Westerflier - Boudewijn Rempt :license: GNU free documentation license 1.3 or later. .. _gitlab : https://invent.kde.org .. _repository : https://invent.kde.org/kde/krita .. _bugzilla : https://bugs.kde.org/ .. _Krita developer IRC : https://krita.org/irc/ .. _API guide : https://api.kde.org/extragear-api/graphics-apidocs/krita/html/index.html .. _intro_hacking_krita: ============================= Introduction to Hacking Krita ============================= .. contents:: Getting started with KDE Software --------------------------------- Krita is a great place to start even if you are brand new to KDE development. We'd love to have you join! You'll be able to work on one of the coolest and fastest-growing open source painting programs out there. Krita also benefits from a modular architecture and the use of the KDE Frameworks and Qt libraries, which makes it easier to focus on new features instead of reinventing the wheel. And it makes coding fun! To work on Krita, you have to use C++ and Qt. It's a good way to learn both, actually! KDE has undergone big changes since a major `2014 reorganization `_ . As a result, working with KDE software has never been easier. Unfortunately, since the changes were so widespread, the documentation has not caught up at all. If you are embarking on this journey, it would be very generous to share your discoveries with others and update pages. (= Getting Started --------------- Here's some links to get your started. #. Most important, the `repository`_. There is a `mirror on Github `_, however note that we do not use Github for development, do not create pull requests or file issues on github. #. KDE Developer wiki - The KDE Techbase Wiki has instructions for new developers. On top of basic tools like C++, git, and general notions such as building software libraries, some special tools that are particular to Krita are Qt, CMake, and KDE Frameworks. It can be very helpful to get started by finding some of the articles discussing these tools and reading up. Here are some of the more useful pages to get you started: * https://techbase.kde.org/Development * https://techbase.kde.org/Contribute * https://techbase.kde.org/Development/Git/Configuration * https://techbase.kde.org/Development/Tutorials * http://flossmanuals.net/kde-guide * http://doc.qt.io/ Qt has some of the best documentation of any software library. #. Set up your development environment and build Krita! #. Find a few bugs to fix in `KDE's Bugtracking system `_. It's often a good idea to get some experience with the code through fixing bugs, to get familiar with the development process without being overwhelmed. Though there's nothing against working on that cool feature that scratches your itch! #. If you intend to be a regular contributor to Krita, even just for bugreports and feature discussion, the first thing you will want to do is register for a `KDE Identity account `_. This serves as your mostly-universal login to KDE code repositories and websites. Building Krita -------------- .. image:: /images/en/cat_guide/Krita-building_for-cats_intro_by-deevad.jpg To get started, all you need to do is get a copy of Krita and build it! This is not all that much much different from building something off GitHub... except that Krita is a very large compared to most software. There are :ref:`build guides ` to get you going on various platforms, but of course Linux is easiest. Working with the Krita codebase ------------------------------- Here's some pointers for working with our codebase. Architecture The code base changes all the time with Krita, we're not afraid of big refactorings, so there is no up to date documentation on the code architecture. There have been some written in the past, but they quickly became outdated and of little use. There is a fairly up to date `API guide`_ if you want to look at how the code is structured. Integrated Development Environment (IDE) The most popular IDEs that we use are Qt Creator, Emacs, KDevelop, or vim. Qt Creator has the advantage of the ctrl-k menu, which lets you leap to classes, lines, everywhere. You don't have to build with Qt Creator though! It can be easier to jump to the terminal, do a 'make', check what's up, and then jump back to the IDE. Resources The most important step to learning the code is to really understand memory management: pointers, smart pointers and pointer arithmetic. This is something that Java and C# developers will need to spend a little more time understanding. Here are a couple resources to get you more familiar with C++ and Qt: * `Qt Concepts `_ * `Design Patterns with Qt `_ * C++ in a Nutshell by O'Reilly (book) Debugging ~~~~~~~~~ There are large and small problems. For small problems the debugger in Qt Creator (run external application) or adding qDebug messages to the code is fine. If the problem is difficult, the first step should always be to write a unit test. A small bit of code that follows a set pattern and exercises the faulty code and shows the problem. That helps so much figuring out a fix and keeping it fixed. When you run a debug build of Krita, you may be surprised how little debug output you see. This is because most of Krita's debugging information is turned off by default. The debug statements are grouped into categories such as ``dbgUI``, ``dbgKrita`` and so on. The output categories are controlled by an environment variable ``QT_LOGGING_RULES``. The list of Krita's debug categories is contained in ``kis_debug.h`` and ``main.cc``, and the rules for the environment variable are described in the `Qt reference for QLoggingCategory `_. As an example, to enable most of Krita's debug output, you can run the following: ``export QT_LOGGING_RULES="krita*=true"; krita`` Using the rule ``*=true`` will produce a firehose, if you want it. Tips when Tackling Issues ~~~~~~~~~~~~~~~~~~~~~~~~~ Features and Refactorings Sometimes you just know that a lot of work is going to be needed to reach a particular goal. These will go in separate feature branches off 'master'. Performance Improvements Sometimes you don't feel like working on a feature -- or someone mentioned something being particularly slow. The first thing to do then is carry out that scenario when Krita runs under `callgrind `_ and `vtune `_. These tools show bottlenecks at the end of a run. It's important to use both, since both give different insights! Bugs Sometimes you rummage around the bugs on b.k.o to see what looks like a nice Saturday morning fix. Sometimes a bug is really urgent (like all data loss bugs). Sometimes someone on IRC or the forum mentions a bug. The first thing to do is reproduce it. The second thing is to look in the code to see what is going on. If it's a crash bug, especially one that seems mysterious, it might help to google for a few of the key lines in the backtrace. Sometimes it's a distribution issue! Blockers If you are helping with Krita and your progress is being blocked by something - let us know! Talk with us on the `Krita developer IRC`_ and we will see what we can do to help! Calligra and Krita ~~~~~~~~~~~~~~~~~~ In October 2015, the Krita project separated from the rest of the Calligra office suite. The new repository still clearly contains this history. Most source code files will have one of two prefixes. "Ko" stands for KOffice, the original name of Calligra office suite. These files mostly comprise basic, lower-level libraries. "Kis" stands for KImageShop, the original name of Krita. These files are where most of the painting-specific functionality is maintained. Krita 2.9 stable is built from the Calligra repo. Krita 3.x and above is built from the Krita repo. Style guidelines ~~~~~~~~~~~~~~~~ See ``HACKING`` in the codebase. Development Philosophy ~~~~~~~~~~~~~~~~~~~~~~ Krita is nearly ten years old, consists of something like a million lines of code, and has had many individual contributors throughout the years. If you run into something in the code that doesn't make sense to you, it may very well not make sense to anyone. Developing a codebase this large is an art form, you should feel confident in making risky changes even if you're not sure they'll work, you can always go back with ``git checkout -- *`` if you mess it up! Getting in Touch ---------------- If you're working on a bug fix, or maybe a bit of GUI polish, you might get stumped. The best thing to do then is to get in touch with the rest of the Krita team. Part of the fun of working on an open source application is the community, after all! Join us on ``#krita on irc.freenode.net`` (keep in mind that most people are in Europe or India) and just ask your question. Stay around, especially if you don't get an answer immediately. Some of the developers have their irc client open permanently and will often answer questions hours later! You can also send mail to the mailinglist: ``kimageshop@kde.org``. It's better not to send mail to individual developers directly, you might accidentally pick someone who hasn't got the answer, and miss the chance of getting your question answered by another Krita developer. Contributing Patches -------------------- .. edit me! add links to techbase for gitlab, not sure if they're written yet. Patch review and development tracking happens on `gitlab`_. To log in, enter your KDE Identity in the LDAP login field. You can join the `Krita: Next `_. If you are used to Github, `the transition to gitlab is not difficult `_, but it is slightly different. To push to invent.kde.org, you will not need to have SSH access setup, but you do KDE identity account. If several of your merge requests are accepted, you can get a commiter's account, which will allow you to push directly to the repositories. You can read more about that here: `Getting a developer account `_ .. attention:: Since moving to the gitlab instance, we don't use ``git@git.kde.org:krita`` but rather ``git@invent.kde.org:kde/krita``. Gitlab will not be able to see your commits if you push to the former. You can use ``git remote set-url origin git@invent.kde.org:kde/krita`` to get everything pointing correctly. So then, how does an aspiring contributor submit patches?: +.. _forking_gitlab: + Forking on Gitlab ~~~~~~~~~~~~~~~~~ .. note:: Work In Progress. #. Forking on gitlab is done by going to the `repository`_ and pressing :guilabel:`fork`. You will then make a personal fork of the repository. #. In your fork, you press :guilabel:`clone` to get the git urls to do the ``git clone`` from. You can then pull and push your commits from these. You can also use the :guilabel:`Web IDE` to make your changes on invent.kde.org, but because Krita is a cpp program, we don't recommend this outside of typo fixes and doxygen. You wouldn't be able to see the effect of your changes, after all! #. Make your first fix, push everything to your fork. #. Once you're done, go to :menuselection:`merge requests` and press :guilabel:`new merge request` #. Tell us what you've done in detail. -The Krita developers be informed of new merge requests, and will try to review your request as soon as possible. If you suspect your patch slipped through the cracks, don't hesitate to make contact us through the means described above. +The Krita developers be informed of new merge requests, and will try to review your request as soon as possible. If you suspect your patch slipped through the cracks, don't hesitate to make contact us through the means described above. Check the :ref:`patch_review_guide` for further guidance. .. https://forum.kde.org/viewtopic.php?f=288&t=125955 diff --git a/untranslatable_pages/modern_cpp_in_krita.rst b/untranslatable_pages/modern_cpp_in_krita.rst index a482f4606..b5c1b6d1f 100644 --- a/untranslatable_pages/modern_cpp_in_krita.rst +++ b/untranslatable_pages/modern_cpp_in_krita.rst @@ -1,325 +1,327 @@ .. meta:: :description: Guide to using features from C++11, C++14 and beyond in Krita's codebase. .. metadata-placeholder :authors: - Michael Abrahams - Boudewijn Rempt - Wolthera van Hövell tot Westerflier :license: GNU free documentation license 1.3 or later. +.. _modern_cpp_in_krita: + ================================================== Modern C++ usage guidelines for the Krita codebase ================================================== .. contents:: General links about using Modern C++ in Qt ------------------------------------------ There have been a few links discussing mixing C++11 with Qt, and starting with Qt 5.6 C++11 support will be default. *Note:* there is a lot of hype about C++11, and although many of its new features are quite welcome, often the trade-offs from these changes get neglected. * `ICS.com `_ * `qt.io `_ * `woboq.com: c++11 in Qt5 `_ * `woboq.com: c++14 in Qt5 `_ * `FOSDEM 2013 presentation slides `_ Here are some more general purpose guides to C++11 features. * `C++11 FAQ `_ - the grand daddy * `Older, more thorough introductions to several topics `_ Qt's API design principles do not always overlap with the C++ Standards Committee design principles. (Range-based for demonstrates the design clash pretty clearly.) * https://wiki.qt.io/API_Design_Principles Particular Features ------------------- Under "drawbacks," every item should list: "Programmers will face another feature they must learn about." Type Inference (auto) ~~~~~~~~~~~~~~~~~~~~~ Motivation: If a function ``f`` has a return type Type, it is redundant to write a local variable ``Type x = f(y).`` Using auto declarations is a simplification in two ways scenarios. First, it allows the programmer to write code without worrying about doing the manual type deduction, for example: .. code:: cpp for( KoXmlReader::const_iterator x = iter.begin(),... ) { } versus: .. code:: cpp for (auto x = iter.begin(), ...) { } This is particularly useful with nested template types and C++11 lambdas, and other complex types which have an obvious role, but a lengthy type definition. A second important benefit of auto is that it allows the programmer to more easily refactor. Suppose we have a function ``gimmeSomeStrings()`` which returns a ``QList``, and we access it somewhere else like this .. code:: cpp auto someStrings = gimmeSomeStrings(); If we later decide that we want to store a hash of strings and that ``gimmeSomeStrings`` should return a ``QMap``, we probably won't need to make any changes inside the client snippet if we are doing tasks like iterating. Drawbacks: the use of auto is be obfuscating. For example, ``auto x = 2`` is not obviously an integer, and ``auto x = {"a", "b", "c"}`` returns ``std::initializer_list``, and sometimes it is not clear what some function returns by the name of the function. Recommendation: Do not use auto, except, maybe, in loops, where there can be no confusion about the type of what is looped. But even there, hesitate. Range-based for loop ~~~~~~~~~~~~~~~~~~~~ Motivation: This is something a long time coming in C++. It is a standardized replacement for Qt's foreach() construct, which works not only with Qt objects but all iterable C++ types. .. code:: cpp for (T x : list ) { ... } It will work with standard tooling and static analysis, and can be faster by defaulting to in-place access. For this reason range-based iterators should always be used for STL containers, if those are ever needed in Krita. Drawbacks: By default, Qt's foreach rewites the code to make a shallow copy and then use const accessors, while c++11 does the opposite, avoiding copying when possible. When using const accessors, this is faster, but if you try to make changes to the data, this will `slow your loop down instead `_. Recommendation: Sometimes, the range-based for is faster. Sometimes the Qt iterator is faster. Personally I like the range-based for in principle, since it works better with static analysis, it has a faster best-case speed, and it is always possible to write it in a way that replicates the ``foreach()`` behavior, though the reverse is not true. On the other hand, there is a bad, dangerous worst case performance hit when a detach/copy is triggered, and this is not easy to catch with standard syntax. In the blog post linked above, the discussion explains that is possible to get around this limitation by defining a macro ``const_()``, which will gives a new syntax to request the compiler use constant iterators: .. code:: cpp for (T x : _const(list) ) { ... } Qt's recommendation on the other hand is to use foreach() for Qt iterators, and range-based for on STL containers, because you always know what you're getting, and you always keep your syntax easy to read. In my opinion is the most meaningful new feature without any sort of clear answer, and quite interesting to think about. General Initializer Lists ~~~~~~~~~~~~~~~~~~~~~~~~~ Motivation: Initializer lists are intended to work in many different places to simplify the syntax for complicated initialization. For example, a list of strings could be initialized ``const QStringList x = {"abc", "def", "xyz" };`` and if you later changed the type to ``QVector``, or even ``std::list``, you wouldn't have to make any change to the right hand side. A second place initializer lists are used is in creating standard initial values for class members. This takes the place of writing a lengthy constructor list like: .. code:: cpp Type::Type() : MemberString1("a") , Subclass1(0) , Subclass2(1) , ... In addition to being more concise, it saves you from repeating yourself, if you have several constructors which all start with the same defaults. Mixed uniform initialization is a separate new feature of initializer lists when constructing classes. It is possible to specify some defaults when you declare member variables, but then override them with delegating constructors. `This MSDN page is a good reference `_. Drawbacks: None I can think of. This is super simple, completely obvious to read and write, and shortens code by removing long unnecessary lists of defaults. Recommendation: Yes! Lambdas, and new-style signals/slots ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Motivation: Lambda expressions are a big new addition for C++11. Many programmers claim they start to feel like an essential part of the language very quickly. One of the biggest uses for lambdas is in the standard algorithm library , which is described below. In Qt5, this, along with std::function and std::bind, allow for One of the most useful C++11 integrations, a new signal/slot syntax which replaces the moc macros SIGNAL() and SLOT() with standard C++. Old style: .. code:: cpp connect(sender, SIGNAL (valueChanged(QString,QString)), receiver, SLOT (updateValue(QString)) ); New style: .. code:: cpp connect(sender, &Sender::valueChanged, receiver, &Receiver::updateValue ); New style signals and slots provide a great benefit from the tooling perspective: now, all types for functions and function arguments can be checked statically, and you don't have to catch typos by monitoring debug messages saying "no such slot." Another possibility is to use lambdas directly inside connect(), instead of defining a class member function which is only used once. The greatest benefit is that the function can be defined right where it is used; it also aids readability to get rid of a list of tiny helper functions from the header. * `"Qt5: C++11 lambdas are your friend" `_ * `C++ language reference `_ * `Qt.io New Signal/Slot Syntax `_ Also gives detailed pros/cons. Drawbacks: The new-style syntax makes it somewhat harder to use default arguments, which requires the use of lambdas. It is also perhaps a little less pretty. Lambdas in general are have become one of the most clunky pieces of C++11 notation. Since they allow a great deal of options for example, capturing by reference with ``[&]`` and capturing by value with ``[=]``, they are a significant new addition to the C++ learning curve. Using small local functions with uninformative names like ``auto F = [&] ( x ) { whatever }`` is confusing for everyone. Although it is possible to use lambdas are tricky inside signals and slots, there are gotchas. Lambdas will not disconnect automatically, although there is a special syntax to make that happen. Recommendation: Lambdas will feel strange to many C++ programmers. At a minimum, any time you use them you should add a comment explaining what you're doing. (Krita codebase could use more comments anyway.) New style signals and slots should be used with caution, especially while the 2.9 branch is being maintained. Overall, the Qt wiki gives a good overview, and I agree with its suggestions, which is to permit a small amount of mixing of the different syntax. Their recommendation is to use new-style signals and slots when possible, which is the vast majority of the time, to fall back on the old macros when one needs to use a default argument, and to use lambdas very rarely, only in cases when one needs to create a signal that is not bound to a particular object. The latter sort of case is not something that C++ newcomers would want to be touching anyway. constexpr ~~~~~~~~~ Motivation: Performing calculations at compile time can speed things up at runtime. `KDAB: speed up your Qt 5 programs using C++11 `_ Drawbacks: Not easy to use these features. Recommendation: This could be useful in specific places, like KoCompositeOpRegistry. Overall it is not something most programmers will run into. ~~~~~~~~~~~ Motivation: A handwritten loop that looks for occurences of the number 20 and replaces it with 99 is routine, and will take several lines to write, including defining local variables. Instead, something like .. code:: cpp std::replace (myvector.cbegin(), myvector.cend(), 20, 99); is more concise, safer is even self-documenting, since the name of the function itself explains what it is doing. If you make sure to use Qt's const iterators, there should never see a performance penalty compared to a hand-written loop, there can sometimes even see a gain. `A list of standard algorithms can be found here. `_ Historically Qt provided its own algorithm library, but now encourages programmers to use the STL versions instead, and Qt's own algorithm library will mostly become deprecated. http://doc.qt.io/qt-5/qtalgorithms.html Unlike range-based for, where it is difficult to specify a const iterator instead of a standard iterator, with ```` we are easily able to specify the const iterator. Drawbacks: Some of the standard algorithms are not completely obvious from observing the name. For example, I could not personally list what are the five arguments of ``std::replace_copy`` off the top of my head, and you shouldn't expect anyone to. When values inside the container need to be modified, non-const iterators may be slower than a Qt foreach() loop. Recommendation: Encourage the use of when it improves code clarity. Speed not a big problem most of the time, don't make changes which are hard to understand just for a tiny hypothetical speed boost. However, moving to and away from Qt foreach() inside hot paths could prove useful in the future. enum class ~~~~~~~~~~ Motivation: These are a type-safe version of enums, and allows the programmer to associate several different types of data with an enum, such as a character. This gives stricter type safety, for example, when it might be possible to accidentally convert a variable into a numeric type. For example: .. code:: cpp enum class Color : char {Red = 'R', Green = 'G', Blue = 'B'}; Other benefits of enum classes are that they can be forward-declared, and that the data can be any sort of constexpr. For example, if one had a constexpr function ``color_symbol()`` that returned the symbol given some color data, the enum class members could be defined like: .. code:: cpp enum class Color: char {Red = color_symbol({255, 0, 0}) ...}; The standard C++ reference does a nice job explaining these features. http://en.cppreference.com/w/cpp/language/enum Drawbacks: Virtually none. Very small changes to the code, more type safety, removes the need for some tables of values. The only problem is sometimes this requires fixing code that was unsafe to begin with. Recommendation: Use freely. Local type definitions (i.e. using) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Motivation: An easier and localized way to use typedefs. Can be at the namespace, class, or function level. Allows you to rewrite a typedef so that the new name occurs on the left hand side of the equals sign, which is easier to read. They allow you to place typedefs closer to where they're used. They are particularly nice inside templates. Drawbacks: Very few. These are quite intuitive Recommendation: Go for it. nullptr ~~~~~~~ Motivation: The use of nullptr as a default pointer initializer is a very small change in C++11, and mostly an aesthetic one. Technically, there are only a few things it prevents : it cannot be converted to a numeric type like ``int x = nullptr;``, and it cannot be used as a class type in a template, so the following is a compiler error: .. code:: cpp meta_type; meta_type x; The most important to nullptr is simply that you are tagging your code - ''hey: there is a null pointer lurking around here, be careful!'' Drawbacks: It takes longer to type nullptr than it takes to type 0, and it's not so visually pleasing. Converting the existing code base would be fairly laborious. Tiny benefits. Recommendation: 100% up to the maintainer's preferences. Deleted, default, override, final ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Motivation: These are keywords used for designing inheritance patterns. They are useful for preventing accidental copy construction. Drawbacks: Since Krita does not export libraries, most of the time we won't need to worry about these. They are limited to solving some pretty specialized problems. Recommendation: No reason to hold back from these features if they seem useful. They are well named and fairly self-explanatory, especially for developers with a Java or C# background. If you apply them correctly, you can prevent other coders from making mistakes when they use your classes. For others, these definitions can be ignored until they cause a compile error, which tell you that you're doing something the wrong way. unique_ptr/QScopedPointer ~~~~~~~~~~~~~~~~~~~~~~~~~ Motivation: `Here is a glowing review of unique_ptr `_. This is really about a philosophy of C++ memory management, not just a particular smart pointer type. The idea is that whenever you create an object on the heap, you should *always* house it inside a smart pointer. The reason this philosophy is considered new to C++11 is that unique_ptr is the first time they 'got it right' designing a very nice smart pointer class. Most importantly, the class uses negligible overhead. In particular: ``sizeof(unique_ptr) = size_t``, it can be passed as a function argument without copying, and dereferencing is inline. QScopedPointer is essentially the same thing as unique_ptr, and perhaps it is more idiomatic to use QScopedPointer instead. .. Note:: It is a useful idiom to store a d-ptr using `QScopedPointer`, but if you do this you must also declare a destructor in the header file, even if it has an empty implementation in the source file. `"Rule of Zero": more about the C++ design philosophy behind unique_ptr. `_ Drawbacks: The philosophy mentioned above can be summarized like this: we should state up front what we are going to prohibit programmers from doing. Like the const keyword, unique_ptr puts restrictions on what can be done with the pointer, the main one being, it cannot be copied. Like enforcing const correctness, this can be annoying to get right throughout a codebase. One particular limitation is that Qt container classes. For example ``QVector`` is invalid, because QVector requires its members can be copied. This makes converting to unique_ptr a bit slow, since ``QVector`` will have to be converted to ``std_array>``. If the owner was being copied before, it will become uncopiable. This can be a good thing, but it can also be extra work. `Moving a unique_ptr requires additional semantics. `_ Recommendation: Smart pointers are already prevalent in the codebase with the KisSP family, but more use of them should be encouraged. d_ptrs should be wrapped in a QScopedPointer. The rule is: first Krita's shared pointers, then Qt's, do not use the std smart pointers. Performance-related (rvalues) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Using move constructors and rvalues are very subtle and advanced features, but widely celebrated as successes of C++11. The point of these features is to save on costs of copying memory when passing function arguments. The idea is that if one is OK allowing a function to steal, alter or destroy its argument, then that function can be called slightly faster if the argument is not copied at all, but instead simply performing an ownership transfer. C++ programmers should already be aware that writing performant code where data gets shuffled around sometimes requires opening a can of ampersands. These features will naturally stay confined to the corners of the codebase behind the scenes where they belong, and should be introduced when they are useful. * `Tutorial for rvalue references `_ * `KDAB: speed up your Qt 5 programs using C++11 `_ * `Slide 37 describes lvalue/rvalue types in exact detail `_ Also explains the terms "xvalue" and "prvalue" sometimes seen as well. Move Constructors ''''''''''''''''' Recommendation: Use whenever it aids performance. Reference Qualifiers (rvalue references) '''''''''''''''''''''''''''''''''''''''' Recommendation: Use whenever it aids performance. C++11 features mostly for template programming ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Krita makes very light use of templates. These features are useful, coming across them in the code base will add complexity for new learners, and have not been necessary so far. * decltype : this is the most likely of these features to be useful, for example, in KisInputManager. * static_assert * variadic templates Other C++11 features that will not be useful ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ * Threading support (Relies on C++ threading model; use Qt threading instead) * shared_ptr and weak_ptr (Relies on C++ threading model; use KisSharedPointer instead) * New literal types (already have QString/ki18n) * Extended Unions (already have QVariant) diff --git a/untranslatable_pages/new_features.rst b/untranslatable_pages/new_features.rst index 1b349e35c..e074bb4db 100644 --- a/untranslatable_pages/new_features.rst +++ b/untranslatable_pages/new_features.rst @@ -1,152 +1,152 @@ .. meta:: :description: Guide to the development of new features. .. metadata-placeholder :authors: - Boudewijn Rempt - Wolthera van Hövell tot Westerflier :license: GNU free documentation license 1.3 or later. .. _developing_features: =================== Developing Features =================== There's several stages to making a feature request become reality. The first section of this page goes over a set of common issues with making feature requests and gives hints on how to make a simple feature request into a proper proposal. The rest documents how a feature goes from a proposal to an actual reality. ------------------------- Step 1: Making a proposal ------------------------- *“vOpenBlackCanvasMischiefPhotoPaintStormToolKaikai has a frobdinger tool! Krita will never amount to a row of beans unless Krita gets a frobdinger tool, too!”* The cool thing about open source is that you can add features yourself, and even if you cannot code, you talk directly with the developers about the features you need in your workflow. Try that with closed-source proprietary software! But, often, the communication goes awry, leaving both users with bright ideas and developers with itchy coding fingers unsatisfied. This post is all about how to work, first together with other artists, then with developers to create good feature requests, feature requests that are good enough that they can end up being implemented. For us as developers it’s sometimes really difficult to read feature requests, and we have a really big to-do list (600+ items at the time of writing, excluding our own dreams and desires). So, having a well written feature proposal is very helpful for us and will lodge the idea better into our conscious. Conversely, a demand for a frobdinger tool because another application has it, so Krita must have it, too, is likely not to get far. Writing proposals is a bit of an art form in itself, and pretty difficult to do right! Asking for a copy of a feature in another application is almost always wrong because it doesn’t tell us the most important thing: What we primarily need like to know is HOW you intend to use the feature. This is the most important part. All Krita features are carefully considered in terms of the workflow they affect, and we will not start working on any feature unless we know why it is useful and how it is exactly used. Even better, once we know how it’s used, we as developers can start thinking about what else we can do to make the workflow even more fluid! Good examples of this approach can be found in the pop-up palette using tags, the layer docker redesign of 3.0, the onion skin docker, the time-line dockers and the assistants. Feature requests should start on the forum, so other artists can chime in. What we want is that a consensus about workflow, about use-cases emerges, something our UX people can then try to grok and create a design for. Once the design emerges, we’ll try an implementation, and that needs testing. For your forum post about the feature you have in mind, check this list: * It is worth investigating first if the feature in question has similar functionality in Krita that might need to be extended to solve the problem. We in fact kind of expect that you have used Krita for a while before making feature requests. Check the manual first! * If your English is not very good or you have difficulty finding the right words, make pictures. If you need a drawing program, I heard Krita is pretty good. * In fact, mock-ups are super useful! And why wouldn’t you make them? Krita is a drawing program made for artists, and a lot of us developers are artists ourselves. Furthermore, this gets past that nasty problem called ‘communication problems’. (Note: If you are trying to post images from photobucket, pasteboard or imgur, it is best to do so with [thumb=800][/thumb]. The forum is pretty strict about image size, but thumb gets around this) * Focus on the workflow. You need to prepare a certain illustration, comic, matte painting, you would be (much) more productive if you could just do --- whatever. Tell us about your problem and be open to suggestions about alternatives. A feature request should be an exploration of possibilities, not a final demand! * The longer your request, the more formatting is appreciated. Some of us are pretty good at reading long incomprehensible texts, but not all of us. Keep to the ABC of clarity, brevity, accuracy. If you format and organize your request we’ll read it much faster and will be able to spent more time on giving feedback on the exact content. This also helps other users to understand you and give detailed feedback! The final proposal can even be a multi-page pdf. * We prefer it if you read and reply to other people’s requests than to start from scratch. For animation we’ve had the request for importing frames, instancing frames, adding audio support, from tons of different people, sometimes even in the same thread. We’d rather you reply to someone else’s post (you can even reply to old posts) than to list it amongst other newer requests, as it makes it very difficult to tell those other requests apart, and it turns us into bookkeepers when we could have been programming. Keep in mind that the Krita development team is insanely overloaded. We’re not a big company, we’re a small group of mostly volunteers who are spending way too much of our spare time on Krita already. You want time from us: it’s your job to make life as easy as possible for us! So we come to: **Things That Will Not Help.** There’s certain things that people do to make their feature request sound important but are, in fact, really unhelpful and even somewhat rude: “Application X has this, so Krita must have it, too”. See above. Extra malus points for using the words “industry standard”, double so if it refers to an Adobe file format. We honestly don’t care if application X has feature Y, especially as long as you do not specify how it’s used. Now, instead of thinking ‘what can we do to make the best solution for this problem’, it gets replaced with ‘oh god, now I have to find a copy of application X, and then test it for a whole night to figure out every single feature… I have no time for this’. We do realize that for many people it’s hard to think in terms of workflow instead of “I used to use this in ImagePainterDoublePlusPro with the humdinger tool, so I need a humdinger tool in krita” --- but it’s your responsibility when you are thinking about a feature request to go beyond that level and make a good case: we cannot play guessing games! “Professionals in the industry use this”. Which professionals? What industry? We cater to digital illustrators, matte painters, comic book artists, texture artists, animators… These guys don’t share an industry. This one is peculiar because it is often applied to features that professionals never actually use. There might be hundreds of tutorials for a certain feature, and it still isn’t actually used in people’s daily work. “People need this.” For the exact same reason as above. Why do they need it, and who are these ‘people’? And what is it, exactly, what they need? “Krita will never be taken seriously if it doesn’t have a glingangiation filter.” Weeell, Krita is quite a serious thing, used by hundreds of thousands of people, so whenever this sentence shows up in a feature request, we feel it might be a bit of emotional blackmail: it tries to to get us upset enough to work on it. Think about how that must feel. “This should be easy to implement.” Well, the code is open and we have excellent build guides, why doesn’t the feature request come with a patch then? The issue with this is very likely it is not actually all that easy. Telling us how to implement a feature based on a guess about Krita’s architecture, instead of telling us the problem the feature is meant to solve makes life really hard! A good example of this is the idea that because Krita has an OpenGL accelerated canvas, it is easy to have the filters be done on the GPU. It isn’t: The GPU accelerated canvas is currently pretty one-way, and filters would be a two-way process. Getting that two way process right is very difficult and also the difference between GPU filters being faster than regular filters or them being unusable. And that problem is only the tip of the iceberg. Some other things to keep in mind: * It is actually possible to get your needed features into Krita outside of the Kickstarter sprints by funding it directly via the Krita foundation, you can mail the official email linked on krita.org for that. * It’s also actually possible to start hacking on Krita and make patches. You don’t need permission or anything! * Sometimes developers have already had the feature in question on their radar for a very long time. Their thinking might already be quite advanced on the topic and then they might say things like ‘we first need to get this done’, or an incomprehensible technical paragraph. This is a developer being in deep thought while they write. You can just ask for clarification if the feedback contains too much technobabble… * Did we mention we’re overloaded already? It can easily be a year or two, three before we can get down to a feature. But that’s sort of fine, because the process from idea to design should take months to a year as well! To summarize: a good feature request: * starts with the need to streamline a certain workflow, not with the need for a copy of a feature in another application * has been discussed on the forums with other artists * is illustrated with mock-ups and example * gets discussed with UX people * and is finally prepared as a proposal * and then it’s time to find time to implement it! * and then you need to test the result. ----------------------------- Step 2: Triaging the proposal ----------------------------- This is strictly a developer task. What is done is that we identify how much work a proposal would need to be implemented. Since 2016 we use these groups to categorize wishbugs so we can plan them into a current release or select them for a fundraiser. To fulfill this step, we need to have a full list which consolidated the ideas and requirements. A good feature request from step one will have these lined out. WISHGROUP: Pie-in-the-sky not going to happen, but it would be really cool. WISHGROUP: Big Projects needs more definition, maybe two, three months of work. WISHGROUP: Stretchgoal up to a couple of weeks or a month of work. WISHGROUP: Larger Usability Fixes maybe a week or two weeks of work. WISHGROUP: Small Usability Fixes half a day or a day of work. WISHGROUP: Out of scope too far from our current core goals to implement. WISHGROUP: Needs proposal and design needs discussion among artists to define scope first. A good proposal doesn't need this. ------------------------------ Step 3: Discussing in irc/phab ------------------------------ Again, strictly a developer task. While nothing stops an adventurous programmer from just going in and implementing something, it helps to go to the #krita irc on freenode and tell us you're working on it. Not because you need permission(Krita is open source after all), but we do want to be able to help you in your endeavours. Such help can include technical help, like where things are in the code, but also interface design help. Some features, such as new frame types for animation, or multithreading on some part or the other also needs careful discussion so we know what is going to need changes. Eventually, a phabricator task will be made to track the issue as well as including mockups. Branch progress is also discussed during the weekly meeting in the irc. -------------------------------- Step 4: Work in a feature branch -------------------------------- New feature branches are called 'name/number-shortdescription'. Examples: "rempt/T379-resource-management", "kazakov/hdr-support", "wolthera/edgedetectionfilter", "jounip/T8764-clone-frames". Originally this was lastname only, but some users have an endlessly long last name while others prefer using their kde identity name. The main purpose is to identify who is responsible for the work in the branch. -Work in a feature branch continues till all major elements are done. A review request is done over the whole branch. Sometimes, for UI purposes, people check out the branch to test it. +Work in a feature branch continues till all major elements are done. A :ref:`review request ` is done over the whole branch. Sometimes, for UI purposes, people check out the branch to test it. When the review is accepted, the branch is merged into master for further testing. When such a branch is merged, a mail needs to be sent to kimageshop@kde.org to notify everyone about this, you can do this automatically by adding 'CCMAIL:kimageshop@kde.org' to your merge commit. As Krita's nightlies are based on master that means a binary will be compiled for the master branch with the new feature in at most 24 hours. --------------------------------------- Step 5: Documentation and demonstration --------------------------------------- When a feature hits the master branch, an entry will be written for the draft branch of this very manual. In particular a reference manual entry will be written to ensure some documentation, some bigger features that interact with one another might also receive a tutorial. The people who programmed or designed the feature are encouraged to help with this documentation process(as they know it best), but it is not mandatory. What is appreciated is that the issue or task is assigned to the manual team. Similarly, demonstration videos or images are welcome, as they will be used for the release notes. The release notes for the next big version are `available here `_, come help us with making the page look good! Finally, upon release a stable branch is created for the master branch (often named Krita/versionnumber), and a release is made with the new feature. diff --git a/untranslatable_pages/patch_review_guide.rst b/untranslatable_pages/patch_review_guide.rst new file mode 100644 index 000000000..e58c03e86 --- /dev/null +++ b/untranslatable_pages/patch_review_guide.rst @@ -0,0 +1,69 @@ +.. meta:: + :description: + Guide to creating and reviewing Krita patches in gitlab. + +.. metadata-placeholder + + :authors: - Wolthera van Hövell tot Westerflier + :license: GNU free documentation license 1.3 or later. + +.. _patch_review_guide: + +============================ +Advanced Merge Request Guide +============================ + +Since April 2019, we're using Gitlab to review merge requests and patches to the code. Check :ref:`forking_gitlab` on how to start with making a merge request. + +When to make a merge request +---------------------------- + +There's three times you need to make merge requests. + +#. When you do not have commit access. +#. When the change you are making is huge, like with feature development, large refactors, complex bugfixes. For these we do not fork, but instead make a branch in the main repository in the following format: ``name/number-shortdescription``. Check :ref:`developing_features` for more information. +#. When you are not sure about whether what you did is correct. It is common within the Krita community that even developers with commit access will have a weaning period in which they still make merge requests for each change as they're getting comfortable with the codebase. It is not mandatory to do so at this point, but requests are the best way for us to help one another with writing good code. + +.. commented out till done + + Updating patches + ---------------- + + +.. This is commented out till it's done. + + Merging in patches + ------------------ + + When to do a squash, when to rebase, when to use the merge gui, default config for Krita repository, etc. + + How to update your fork(if using) from master after being done with the merge. + + +Checklist for review +-------------------- + +Here's a quick checklist that is gone over when reviewing patches. While some requests require specialist knowledge, these items are so universal that anyone who knows how to check them can do so and comment on them. Feel free to do this yourself on `open requests `_, we welcome it! + +Also check out the `manual for reviewing merge requests in Gitlab `_. + +Does the code build + Most important check. Apply the patch locally and build it. If it doesn't build, the patch will not be accepted at all. +Does it not crash? + Basically, build the patch, run Krita, and check if the functions associated doesn't crash. If it does, make a backtrace and post it in the review. +Does it leak memory? + .. HOW TO CHECK +Does the patch break tests? + .. HOW TO CHECK +CPP features used. + Is the usage of CPP in accordance with HACKING and the :ref:`modern_cpp_in_krita` guidelines? So for example, is auto not used outside of the single case we accept it? +Is the code in conformance with KDE/Krita style? + Check the HACKING file for directions. +Are the commit messages sensible? + There's several guides for this, but in general, try to make sure that the commit messages actually explain what you did. + + * https://github.com/RomuloOliveira/commit-messages-guide +Does the patch make sense. + This is in the category of specialist knowledge, but you can apply some common sense here. If a patch for a filter also adjusts the resource management, you can ask yourself why this would be necessary. Furthermore, does the patch actually fix the thing it says it is fixing? These are not easy checks to make, but important things to consider when looking at the patch. + +Requests that need changes to them need to be labeled with ``needs-changes``. Requests that are accepted should be labeled ``accepted``. This is to ensure we can figure out which requests are in need of review. Requests that need to be reviewed need to lack both labels.